← Commit history

1.0.41: 1.0.41: the background guard stops guessing. It used to decide whether a KiCad window reaching the foreground was the user's doing from a 120 ms mouse-button poll and a timing window, and while any verb ran it pushed back ANY bridge-owned KiCad window, again and again. John, clicking KiCad's taskbar buttons during a test: 'i just tried clicking to bring kicad to the foreground and then you yank it to the background on me'. New rule, no heuristics: the bridge may push back only the window it is OPENING RIGHT NOW (one that did not exist when the verb in flight began, until 3 s after that verb ends). Every other KiCad window is the user's to raise by click, Alt-Tab, Win+number or anything else, whatever the bridge is doing. After a push it ASKS for the user's window back with one plain SetForegroundWindow: no thread-input attach, no ShowWindow, no BringWindowToTop, never a shell window (1.0.40; forcing the taskbar crashed Explorer in Taskbar.View.dll). Also found: the win/ unit tests never run in the Linux container, and the once-per-window test had been failing unseen since 2026-09-12; the whole Windows test binary (36 tests) now passes on a real Windows box.

John Lauer ·93270fa2d1 ·1mo ago ·parent 533a4fe
2 files changed +59−4
rust/crates/kicad-platform/src/win/etiquette.rs+49−4
@@ -69,6 +69,8 @@ use super::{focus, hwnd, hwnd_u64, is_kicad_exe, is_window, process, responsive}  /// User input this recent, with the cursor on the shell or on the window, is the user's own click. const USER_INPUT_GRACE_SECS: f64 = 1.5;+/// A window created by a verb stays "fresh" (pushable) this long after that verb ends.+const FRESH_AFTER_VERB: Duration = Duration::from_secs(3); /// Sentinel poll period. const POLL: Duration = Duration::from_millis(120); /// A click counts as "the user brought this up" for this long after it landed.@@ -141,6 +143,12 @@ pub struct Transition {     pub foreground_at_verb_start: bool,     /// Already pushed once (the once-per-(hwnd, pid) ledger).     pub already_bounced: bool,+    /// The window the bridge is OPENING RIGHT NOW: it did not exist when the verb in flight began+    /// (or the verb ended under 3 s ago). Only such a window may ever be pushed back. Every other+    /// KiCad window is the user's to raise by click, Alt-Tab, Win+number or anything else, with no+    /// guessing about what caused the foreground change (John, 2026-09-21: "i just tried clicking to+    /// bring kicad to the foreground and then you yank it to the background on me").+    pub fresh: bool,     pub guards: usize,     pub inflight: usize,     pub spawn_watch: bool,@@ -185,6 +193,9 @@ pub fn decide(t: &Transition) -> Verdict {     if t.foreground_at_verb_start {         return Verdict::Leave("baseline foreground (user was already in it, left alone)");     }+    if !t.fresh {+        return Verdict::Leave("an existing window, the user's to raise (left alone)");+    }     // The bridge is the cause (a verb in flight, a guard, our fresh spawn): bounce even if     // this hwnd was bounced before. A box with the foreground lock off (arav-rog) lets a     // KiCad window come forward twice while it loads, and the once-ledger left the second@@ -260,6 +271,11 @@ struct Inner {     guards: Vec<Guard>,     /// KiCad windows that existed when the current run of verbs began.     baseline: HashSet<u64>,+    /// KiCad windows that existed when the verb now in flight began. Refreshed at EVERY verb start+    /// (`baseline` above is only refreshed when no guard is armed, which during back-to-back verbs+    /// is never, so it cannot answer "is this window new").+    verb_baseline: HashSet<u64>,+    last_verb_end: Option<Instant>,     /// The foreground window when the current run of verbs began, if KiCad.     baseline_fg: Option<u64>,     /// The user's most recent non-KiCad foreground window; a bounce hands the activation@@ -654,6 +670,7 @@ fn handle_foreground(h: u64, source: Source, cache: &mut ExeCache) {             owned,             foreground_at_verb_start: (g.inflight > 0 || !g.guards.is_empty()) && g.baseline_fg == Some(h),             already_bounced: g.backgrounded_once.contains_key(&(h, pid)),+            fresh: !g.verb_baseline.contains(&h) && (g.inflight > 0 || within(g.last_verb_end, FRESH_AFTER_VERB, now)),             guards: g.guards.len(),             inflight: g.inflight,             spawn_watch: active_until(g.spawn_watch_until, now),@@ -694,7 +711,11 @@ fn handle_foreground(h: u64, source: Source, cache: &mut ExeCache) {             // without this KiCad keeps the activation and raises itself again.             let user = lock().last_user_fg;             let shell = user != 0 && is_shell_window(user, &mut ExeCache::default());-            let restored = user != 0 && user != h && !shell && matches!(focus::bring_to_front(hwnd(user)), Ok(true));+            // ASK, never force (John, 2026-09-21: "just push the kicad window to the background but you+            // never force the previous window to the foreground"). One plain SetForegroundWindow so the+            // keyboard does not stay in a KiCad window the user cannot see; no thread-input attach, no+            // ShowWindow, no BringWindowToTop, never a shell window. If Windows says no, that is the end.+            let restored = user != 0 && user != h && !shell && focus::request_foreground(hwnd(user));             lock().bounce_holds.insert(h, now + BOUNCE_HOLD);             log(&format!("[bg] {h}: sent behind ({result}); user window {user} {}.", if restored { "restored" } else { "not restored" }));             match source {@@ -1060,6 +1081,9 @@ pub fn begin_verb(verb: &str, spawns: bool) {         g.baseline = kicad_hwnds(&mut cache).into_iter().collect();         g.baseline_fg = if fg_is_kicad { Some(fg) } else { None };     }+    if g.inflight == 0 {+        g.verb_baseline = kicad_hwnds(&mut cache).into_iter().collect();+    }     g.inflight += 1;     if spawns {         g.guards.push(Guard { verb: verb.to_string(), deadline: now + guard_for(verb) });@@ -1075,6 +1099,9 @@ pub fn end_verb(verb: &str) {     let idle = {         let mut g = lock();         g.inflight = g.inflight.saturating_sub(1);+        if g.inflight == 0 {+            g.last_verb_end = Some(now);+        }         if let Some(gd) = g.guards.iter_mut().rev().find(|gd| gd.verb == verb) {             gd.deadline = now + guard_for(verb);         }@@ -1187,7 +1214,18 @@ mod tests {     use super::*;      fn steal() -> Transition {-        Transition { is_kicad: true, owned: true, ..Default::default() }+        Transition { is_kicad: true, owned: true, fresh: true, ..Default::default() }+    }++    #[test]+    fn an_existing_window_is_never_pushed_whatever_the_bridge_is_doing() {+        // A verb in flight, a guard armed, inside the spawn watch, NO click or Alt seen (the poll+        // missed it): the window existed before this verb, so it is the user's to raise.+        let t = Transition { fresh: false, guards: 2, inflight: 1, spawn_watch: true, ..steal() };+        assert_eq!(decide(&t), Verdict::Leave("an existing window, the user's to raise (left alone)"));+        // The window the verb is opening right now is still kept behind.+        let t = Transition { fresh: true, guards: 1, inflight: 1, ..steal() };+        assert!(matches!(decide(&t), Verdict::Bounce(_)));     }      #[test]@@ -1239,11 +1277,18 @@ mod tests {      #[test]     fn once_per_window() {+        // This test failed silently from 2026-09-12 (the "bounce again during a guard" change) until+        // 2026-09-21, because the win/ tests never run in the Linux container. The contract as it+        // really is: with nothing armed a window is pushed at most once; while the bridge is opening+        // it (guard or verb in flight) it may be pushed again, and `fresh` limits that to the one+        // window being opened.         let mut t = steal();-        t.guards = 1;-        assert!(matches!(decide(&t), Verdict::Bounce(_)));         t.already_bounced = true;         assert!(matches!(decide(&t), Verdict::Leave(l) if l.starts_with("already backgrounded once")));+        t.guards = 1;+        assert!(matches!(decide(&t), Verdict::Bounce(_)));+        t.fresh = false;+        assert!(matches!(decide(&t), Verdict::Leave(l) if l.starts_with("an existing window")));     }      #[test]
rust/crates/kicad-platform/src/win/focus.rs+10
@@ -54,6 +54,16 @@ pub fn show_no_activate(h: HWND) -> Result<(), String> { /// GetForegroundWindow reports the target afterwards, not whether the calls returned. /// Minimized windows are restored; anything else keeps its size (SW_RESTORE would /// un-maximize a maximized editor, which the Python version got wrong).+/// Ask Windows to make `h` the foreground window: one SetForegroundWindow and nothing else. No+/// AttachThreadInput, no ShowWindow, no BringWindowToTop. Returns whether Windows agreed.+pub fn request_foreground(h: HWND) -> bool {+    if h.is_invalid() || !super::responsive(h, GATE_MS) {+        return false;+    }+    // SAFETY: a plain request on a valid, responsive window handle.+    unsafe { SetForegroundWindow(h) }.as_bool()+}+ pub fn bring_to_front(h: HWND) -> Result<bool, String> {     require_responsive(h, "bring_to_front", GATE_MS)?;     // SAFETY: reading the foreground handle.