fix: the picker comes forward on Windows, and a picker nobody

crates/linkunbound-mac/src/native.rs

@@ -113,17 +113,19 @@
 }
 
 pub fn take_the_keyboard(view: isize) {
     let Some(mtm) = MainThreadMarker::new() else {
         return;
     };
     let Some(window) = window_of(view) else {
         return;
     };
     let app = NSApplication::sharedApplication(mtm);
     app.setActivationPolicy(NSApplicationActivationPolicy::Regular);
+    // In front of its level even when the activation is refused or still on its way.
+    window.orderFrontRegardless();
     #[allow(deprecated)]
     app.activateIgnoringOtherApps(true);
     window.makeKeyAndOrderFront(None);
 }
 
 #[must_use]

crates/linkunbound-shell/src/main.rs

@@ -78,6 +78,10 @@
         linkunbound_win::front_window()
     }
 
+    pub fn takes_the_front_back() -> bool {
+        true
+    }
+
     pub fn never_activates(window: isize) {
         linkunbound_win::never_activates(window);
     }
@@ -153,6 +157,10 @@
         linkunbound_mac::front_application()
     }
 
+    pub fn takes_the_front_back() -> bool {
+        false
+    }
+
     pub fn never_activates(_window: isize) {}
 
     pub fn shift_is_down() -> bool {
@@ -224,6 +232,10 @@
         0
     }
 
+    pub fn takes_the_front_back() -> bool {
+        false
+    }
+
     pub fn never_activates(_window: isize) {}
 
     pub fn shift_is_down() -> bool {
@@ -452,19 +464,22 @@
     /// Read when the link arrived, not when the user picks: by then the picker
     /// itself is the foreground window, and the rule would bind to us.
     source: Option<String>,
-    /// Links that arrived while one was already on screen. Redressing the window
-    /// under the user would open the wrong one, and dropping them would lose a
-    /// click they already made.
+    /// Links that arrived while one was on screen and being answered. Redressing
+    /// the window under the user would open the wrong one, and dropping them
+    /// would lose a click they already made.
     waiting: std::collections::VecDeque<String>,
 }
 
-/// A link that arrives while one is already on screen waits its turn. Redressing
-/// the window under the user opens the wrong one — they aimed at what they could
-/// see — and discarding it loses a click they already made.
+/// A link that arrives while one is on screen and attended waits its turn. Redressing the
+/// window under the user opens the wrong one — they aimed at what they could see — and
+/// discarding it loses a click they already made. One arriving over a picker nobody is
+/// answering — refused the front, or left behind by the very click that sent the link — takes
+/// its place instead: a picker that never answered was clicked past, and queueing behind it
+/// is how a dozen clicks came to show nothing at all.
 ///
 /// Kept apart from the window so the decision can be checked without one.
-fn claims_the_window(shown: &mut Shown, url: String, occupied: bool) -> Option<String> {
-    if occupied {
+fn claims_the_window(shown: &mut Shown, url: String, attended: bool) -> Option<String> {
+    if attended {
         // A program that retries the same link every second fills the queue with one click; a
         // link already waiting, or the one on screen, is that click.
         let already = shown.url.as_deref() == Some(url.as_str())
@@ -483,10 +498,16 @@
 /// Past this many, the oldest goes: nobody clicks that many links while a picker is up.
 const WAITING_ROOM: usize = 12;
 
-fn present(picker: &Picker, words: &Strings, shown: &Rc<RefCell<Shown>>, url: String) {
-    let occupied = picker.window().is_visible();
-    let Some(url) = claims_the_window(&mut shown.borrow_mut(), url, occupied) else {
-        return;
+/// Only a picker that holds the front is being looked at; one that is up without it is being
+/// clicked past. Shown means Slint's flag, which a window behind another still carries.
+fn attended(picker: &Picker) -> bool {
+    picker.window().is_visible() && native_handle(picker.window()).is_some_and(host::is_in_front)
+}
+
+/// Whether the link went on screen, rather than behind one already being answered.
+fn present(picker: &Picker, words: &Strings, shown: &Rc<RefCell<Shown>>, url: String) -> bool {
+    let Some(url) = claims_the_window(&mut shown.borrow_mut(), url, attended(picker)) else {
+        return false;
     };
     let listed = rows(physical_icon_side(picker));
     if listed.is_empty() {
@@ -502,7 +523,7 @@
             host::keep_off_the_taskbar(handle, corner_of(picker));
             host::take_the_keyboard(handle);
         }
-        return;
+        return true;
     }
     let source = host::clicked_in();
     dress(picker, words, &url, source.as_deref(), &listed);
@@ -539,6 +560,7 @@
             ui.refresh_strip();
         }
     }
+    true
 }
 
 /// The picker is summoned by a click and answered with the keyboard, so it has
@@ -556,13 +578,13 @@
 /// clicked is silently dropped.
 fn next_in_line(picker: &Picker, words: &Strings, shown: &Rc<RefCell<Shown>>) {
     let queued = shown.borrow_mut().waiting.pop_front();
-    if let Some(url) = queued {
-        present(picker, words, shown, url);
-        if let Some(ui) = ui() {
-            // The next link needs its own settle: carrying the previous one's
-            // state would dismiss it on the tick after it appeared.
-            ui.watch_focus();
-        }
+    if let Some(url) = queued
+        && present(picker, words, shown, url)
+        && let Some(ui) = ui()
+    {
+        // The next link needs its own settle: carrying the previous one's
+        // state would dismiss it on the tick after it appeared.
+        ui.watch_focus();
     }
 }
 
@@ -642,13 +664,20 @@
     taskbar_seen: Cell<bool>,
     /// The window the picker was shown over when it could not take the front.
     shown_over: Cell<Option<isize>>,
+    put_up: Cell<Option<std::time::Instant>>,
     #[cfg(target_os = "macos")]
     launch_decided: Cell<bool>,
 }
 
 /// How long a browser gets to bring its window up after being launched.
 const BROWSER_ARRIVES_WITHIN: Duration = Duration::from_millis(1500);
 
+const FRONT_SETTLES_WITHIN: Duration = Duration::from_millis(500);
+
+fn still_settling(put_up: Option<std::time::Instant>) -> bool {
+    put_up.is_some_and(|at| at.elapsed() < FRONT_SETTLES_WITHIN)
+}
+
 thread_local! {
     static UI: RefCell<Option<Rc<Ui>>> = const { RefCell::new(None) };
 }
@@ -822,30 +851,37 @@
     /// Slint offers no focus-lost event, so this polls — but only while the
     /// picker is on screen, and it stops itself the moment it is not.
     fn watch_focus(self: &Rc<Self>) {
         self.held_focus.set(false);
         self.shown_over.set(None);
+        self.put_up
+            .set(host::takes_the_front_back().then(std::time::Instant::now));
         let weak = Rc::downgrade(self);
         self.watch.start(
             slint::TimerMode::Repeated,
             Duration::from_millis(60),
             move || {
                 let Some(ui) = weak.upgrade() else { return };
                 if !ui.picker.window().is_visible() {
                     ui.watch.stop();
                     return;
                 }
                 // Without a handle nothing is known yet, and taking that for
                 // "in front" armed the dismissal before the window ever had the
                 // focus: the picker vanished on the tick after it appeared.
                 let Some(ours) = native_handle(ui.picker.window()) else {
                     return;
                 };
                 // The icon has to say what a click would do, and a modifier held while the
                 // pointer works may produce no key event to learn it from.
                 ui.picker
                     .set_private_on(ui.picker.get_pinned_private() || host::shift_is_down());
                 if host::is_in_front(ours) {
                     ui.held_focus.set(true);
+                } else if still_settling(ui.put_up.get())
+                    && host::clicked_in() == ui.shown.borrow().source
+                {
+                    ui.held_focus.set(false);
+                    host::take_the_keyboard(ours);
                 } else if !ui.held_focus.get() {
                     // An elevated window in front keeps a plain process out of its input, so
                     // the picker sits there unfocused; the window the person was in is
@@ -908,31 +944,34 @@
 /// One link, start to finish, on the UI thread. Called the instant the socket
 /// reads it: waiting for a poll turned four milliseconds of work into sixty.
 fn arrived(raw: String) {
     let Some(ui) = ui() else {
         hold_for_the_loop(raw);
         return;
     };
     // The socket takes a line from any process of this user; the command line is
     // normalised and this has to be too, or the scheme guard is walked around.
     let Some(url) = normalise(&raw) else { return };
     ui.catch_up();
 
     if let Some(fired) = answered_by_rule(&url, host::clicked_in().as_deref()) {
         if store().prefs().notify_on_rule {
             ui.firing.replace(Some(fired.rule_id.clone()));
             flash(&ui.notice, &ui.words.get(), &fired);
             ui.count_down();
             // Shown over an open picker, the notice must not take the digits being typed.
             if ui.picker.window().is_visible()
                 && let Some(handle) = native_handle(ui.picker.window())
             {
                 host::take_the_keyboard(handle);
             }
         }
         return;
     }
-    present(&ui.picker, &ui.words.get(), &ui.shown, url);
-    ui.watch_focus();
+    // A link that only queued leaves the picker's settle alone: restarting it would have the
+    // picker take the front back from wherever the person had just gone.
+    if present(&ui.picker, &ui.words.get(), &ui.shown, url) {
+        ui.watch_focus();
+    }
 }
 
 /// Links the socket took before the loop could: the copy that handed them over believes they
@@ -1051,297 +1090,302 @@
 }
 
 fn main() -> Result<(), slint::PlatformError> {
     let args: Vec<String> = std::env::args_os()
         .map(|a| a.to_string_lossy().into_owned())
         .collect();
     let incoming = link_from(&args);
     // Sign-in and a click on the icon both start this with no link, and only one of them wants a
     // window: the startup task passes this, the Start menu tile does not.
     let hushed = args.iter().any(|a| a == "--hushed");
 
     let _server = match single::claim(handed_to_the_loop) {
         Some(server) => server,
         None => {
             let Some(url) = incoming.as_deref() else {
                 // A second copy at sign-in, or the one the installer relaunched before the old
                 // resident had let go: nobody asked for a window.
                 if !hushed {
                     open_settings();
                 }
                 return Ok(());
             };
             if single::hand_over(url) {
                 return Ok(());
             }
             // A resident that will not take the link is one that is no longer
             // there: the socket it held is free again, and leaving here dropped
             // a click already made.
             match single::claim(handed_to_the_loop) {
                 Some(server) => server,
                 None => return Ok(()),
             }
         }
     };
 
     // Nothing was running, so this copy stays as the resident — and a person who started it by
     // hand, with no link and no tray icon to click, would otherwise see nothing happen at all.
     let plain_launch = incoming.is_none() && !hushed;
     #[cfg(target_os = "macos")]
     let _origins = linkunbound_mac::watch_activations();
     #[cfg(target_os = "macos")]
     let _events = linkunbound_mac::listen({
         let quiet = !plain_launch || !host::is_bundled();
         move |event| {
             let _ = slint::invoke_from_event_loop(move || sent_by_launch_services(event, quiet));
         }
     });
     if plain_launch && !cfg!(target_os = "macos") {
         open_settings();
     }
 
     #[cfg(target_os = "macos")]
     without_taking_the_front()?;
     let picker = Picker::new()?;
     let shown = Rc::new(RefCell::new(Shown::default()));
 
     // Alt+F4 reaches the window as a close request, which Slint answers by hiding it: the queue
     // behind the link then waited for nothing.
     {
         let handle = picker.as_weak();
         picker.window().on_close_requested(move || {
             if let Some(window) = handle.upgrade() {
                 window.invoke_dismissed();
             }
             slint::CloseRequestResponse::KeepWindowShown
         });
     }
     {
         let handle = picker.as_weak();
         let shown = Rc::clone(&shown);
         picker.on_dismissed(move || {
             let Some(window) = handle.upgrade() else {
                 return;
             };
             let _ = window.hide();
             host::let_whoever_opens_next_come_forward();
             if let Some(ui) = ui() {
                 next_in_line(&window, &ui.words.get(), &shown);
             }
         });
     }
     {
         let handle = picker.as_weak();
         picker.on_typed(move |typed, private| {
             let Some(window) = handle.upgrade() else {
                 return false;
             };
             let Some(digit) = typed.chars().next().and_then(host::digit_behind) else {
                 return false;
             };
             let Some(index) = i32::try_from(digit).ok().map(|d| d - 1) else {
                 return false;
             };
             if index >= window.get_rows().row_count().try_into().unwrap_or(i32::MAX) {
                 return false;
             }
             window.invoke_open(index, private);
             true
         });
     }
     {
         let handle = picker.as_weak();
         picker.on_step_reach(move |delta| {
             let Some(window) = handle.upgrade() else {
                 return 0;
             };
             let dead: Vec<bool> = window.get_all_reaches().iter().map(|r| r.dead).collect();
             linkunbound_shell::next_live(&dead, window.get_reach_index(), delta)
         });
     }
     {
         let handle = picker.as_weak();
         let shown = Rc::clone(&shown);
         picker.on_open(move |index, private| {
             let Some(window) = handle.upgrade() else {
                 return;
             };
             // A reach the keys landed on while it was dead is no reach at all.
             let at = window.get_reach_index();
             let dead = usize::try_from(at)
                 .ok()
                 .and_then(|i| window.get_all_reaches().row_data(i))
                 .is_some_and(|reach| reach.dead);
             let reach = if dead { Reaches::Once } else { Reaches::at(at) };
             let held = shown.borrow();
             let Some(url) = held.url.clone() else { return };
             let source = held.source.clone();
             let Some(chosen) = held.rows.get(usize::try_from(index).unwrap_or(0)) else {
                 return;
             };
             // The pointer can do the whole errand with Shift held and never produce a key event,
             // so the modifier is read now rather than recalled from one.
             let private = private || host::shift_is_down();
             host::let_whoever_opens_next_come_forward();
             match linkunbound_core::launch(
                 &catalogue(),
                 &chosen.browser_id,
                 chosen.profile_id.as_deref(),
                 private,
                 &url,
             ) {
                 Ok(()) => {
                     if let Some(ui) = ui() {
                         ui.launched_at.set(Some(std::time::Instant::now()));
                     }
                     let words = ui().map(|ui| ui.words.get());
                     if remember(&url, chosen, private, reach, source) {
                         let _ = window.hide();
                         drop(held);
                         if let Some(words) = words {
                             next_in_line(&window, &words, &shown);
                         }
                     } else if let Some(words) = words {
                         window.set_alarming(true);
                         window.set_problem(words.fail_not_remembered.into());
                     }
                 }
                 Err(why) => {
                     if let Some(ui) = ui() {
                         window.set_alarming(true);
                         window.set_problem(ui.words.get().on_failure(&why).into());
                     }
                 }
             }
         });
     }
     {
         let handle = picker.as_weak();
         let shown = Rc::clone(&shown);
         picker.on_copy(move || {
             let Some(window) = handle.upgrade() else {
                 return;
             };
             let url = shown.borrow().url.clone();
             // The tick only ever animated: nothing had reached the clipboard.
             if let Some(url) = url {
                 window.set_copied(host::copy_text(&url));
             }
         });
     }
     picker.on_update_asked(|| {
         if let Some(ui) = ui() {
             ui.take_update();
         }
     });
     picker.on_update_dismissed(|| {
         if let Some(ui) = ui() {
             ui.put_update_away();
         }
     });
 
     let notice = Notice::new()?;
     {
         let handle = notice.as_weak();
         notice.on_undo(move || {
             if let Some(id) = ui().and_then(|ui| ui.firing.borrow_mut().take()) {
                 forget(&id);
             }
             if let Some(window) = handle.upgrade() {
                 let _ = window.hide();
             }
         });
     }
     {
         let handle = notice.as_weak();
         notice.on_dismissed(move || {
             if let Some(window) = handle.upgrade() {
                 let _ = window.hide();
             }
         });
     }
 
     let spoken = Language::chosen(store().prefs().locale).strings();
     let light_taskbar = host::light_taskbar();
     let tray = Tray::install(light_taskbar, &spoken);
     let (asks, tray_inbox) = channel::<Asked>();
 
     let state = Rc::new(Ui {
         picker,
         notice,
         shown,
         words: Cell::new(spoken),
         firing: RefCell::new(None),
         tray,
         watch: slint::Timer::default(),
         countdown: slint::Timer::default(),
         progress_watch: slint::Timer::default(),
         looker: slint::Timer::default(),
         put_away: RefCell::new(None),
         prefs_seen: Cell::new(prefs_touched_at()),
         held_focus: Cell::new(false),
         launched_at: Cell::new(None),
         light_seen: Cell::new(false),
         taskbar_seen: Cell::new(light_taskbar),
         shown_over: Cell::new(None),
+        put_up: Cell::new(None),
         #[cfg(target_os = "macos")]
         launch_decided: Cell::new(false),
     });
     // Applied here rather than only on a later change: the tray was built
     // visible and the theme never left settings at all.
     state.obey(&store().prefs());
     state.keep_looking();
     UI.with_borrow_mut(|slot| *slot = Some(Rc::clone(&state)));
 
     // Deferred into the loop rather than presented here: before it runs there is no window
     // handle, so the picker would open with a taskbar button and without the keyboard.
     if let Some(url) = incoming {
         let _ = slint::invoke_from_event_loop(move || arrived(url));
     }
     release_to_the_loop();
 
     #[cfg(target_os = "macos")]
     if plain_launch {
         slint::Timer::single_shot(Duration::from_secs(2), || {
             if let Some(ui) = ui()
                 && !ui.launch_decided.get()
             {
                 ui.launch_decided.set(true);
                 open_settings();
             }
         });
     }
 
     // The tray hands its events to a global queue rather than a callback, so
     // this is the one thing still on a clock — and only while the app is idle,
     // which is exactly when nothing else needs the CPU.
     let pump = slint::Timer::default();
     pump.start(
         slint::TimerMode::Repeated,
         Duration::from_millis(120),
         move || {
             if let Some(tray) = state.tray.as_ref() {
                 tray.drain(&asks);
             }
             while let Ok(what) = tray_inbox.try_recv() {
                 if what == Asked::Settings {
                     state.catch_up();
                 }
                 asked_for(what);
             }
         },
     );
 
     slint::run_event_loop_until_quit()
 }
 
 #[cfg(test)]
 mod tests {
     use super::{
-        Listed, Shown, WAITING_ROOM, claims_the_window, link_from, physical, rule_for, with_icons,
+        FRONT_SETTLES_WITHIN, Listed, Shown, UI, Ui, WAITING_ROOM, claims_the_window, host,
+        link_from, next_in_line, physical, present, rule_for, still_settling, with_icons,
     };
     use linkunbound_core::Scope;
-    use linkunbound_core::normalise;
-    use linkunbound_shell::Reaches;
+    use linkunbound_core::{Language, normalise};
+    use linkunbound_shell::{Notice, Picker, Reaches};
+    use slint::ComponentHandle;
+    use std::cell::{Cell, RefCell};
+    use std::rc::Rc;
 
     #[cfg(target_os = "macos")]
     mod launch_services {
@@ -1415,6 +1459,87 @@
         }
     }
 
+    fn headless_ui() -> Rc<Ui> {
+        i_slint_backend_testing::init_no_event_loop();
+        let ui = Rc::new(Ui {
+            picker: Picker::new().expect("a window"),
+            notice: Notice::new().expect("a notice"),
+            shown: Rc::new(RefCell::new(Shown::default())),
+            words: Cell::new(Language::English.strings()),
+            firing: RefCell::new(None),
+            tray: None,
+            watch: slint::Timer::default(),
+            countdown: slint::Timer::default(),
+            progress_watch: slint::Timer::default(),
+            looker: slint::Timer::default(),
+            put_away: RefCell::new(None),
+            prefs_seen: Cell::new(None),
+            held_focus: Cell::new(false),
+            launched_at: Cell::new(None),
+            light_seen: Cell::new(false),
+            taskbar_seen: Cell::new(false),
+            shown_over: Cell::new(None),
+            put_up: Cell::new(None),
+            #[cfg(target_os = "macos")]
+            launch_decided: Cell::new(false),
+        });
+        UI.with_borrow_mut(|slot| *slot = Some(Rc::clone(&ui)));
+        ui
+    }
+
+    #[test]
+    fn a_link_over_a_picker_nobody_answers_takes_its_place_and_the_queue_follows() {
+        let ui = headless_ui();
+        let words = ui.words.get();
+
+        assert!(present(
+            &ui.picker,
+            &words,
+            &ui.shown,
+            "https://one.test/".to_owned()
+        ));
+        ui.watch_focus();
+        assert!(ui.picker.window().is_visible());
+        assert_eq!(ui.shown.borrow().url.as_deref(), Some("https://one.test/"));
+        assert_eq!(ui.put_up.get().is_some(), host::takes_the_front_back());
+
+        assert!(present(
+            &ui.picker,
+            &words,
+            &ui.shown,
+            "https://two.test/".to_owned()
+        ));
+        assert_eq!(ui.shown.borrow().url.as_deref(), Some("https://two.test/"));
+        assert!(ui.shown.borrow().waiting.is_empty());
+
+        let _ = ui.picker.hide();
+        ui.shown
+            .borrow_mut()
+            .waiting
+            .push_back("https://three.test/".to_owned());
+        next_in_line(&ui.picker, &words, &ui.shown);
+        assert!(ui.picker.window().is_visible());
+        assert_eq!(
+            ui.shown.borrow().url.as_deref(),
+            Some("https://three.test/")
+        );
+        assert!(ui.shown.borrow().waiting.is_empty());
+    }
+
+    #[test]
+    fn the_front_is_taken_back_only_in_the_first_moment() {
+        let now = std::time::Instant::now();
+        assert!(still_settling(Some(now)));
+        let stale = now
+            .checked_sub(FRONT_SETTLES_WITHIN * 2)
+            .expect("uptime beyond a second");
+        assert!(!still_settling(Some(stale)), "the moment has passed");
+        assert!(
+            !still_settling(None),
+            "never put up, or a system that never takes it back"
+        );
+    }
+
     /// The screen is measured in physical pixels and the window is described in logical ones, so
     /// on a 150% display the untouched number asks for two thirds of the room it needs and the
     /// picker opens clipped.
@@ -1515,9 +1640,29 @@
         );
     }
 
-    /// A link arriving while one is on screen must not redress the window: the
-    /// user aimed at what they could see. And it must not be dropped either —
-    /// that click already happened.
+    /// A link arriving over a picker that never got the front, or lost it to the click that sent
+    /// the link, takes its place: the person clicked past a window that was not answering, and
+    /// the same link clicked again is that person trying once more, not a program retrying.
+    #[test]
+    fn a_link_arriving_over_a_picker_nobody_is_answering_takes_its_place() {
+        let mut shown = Shown {
+            url: Some("https://behind.test/".to_owned()),
+            ..Shown::default()
+        };
+        let next = claims_the_window(&mut shown, "https://next.test/".to_owned(), false);
+        assert_eq!(next.as_deref(), Some("https://next.test/"));
+        assert!(
+            shown.waiting.is_empty(),
+            "nothing queues behind an unanswered picker"
+        );
+
+        let again = claims_the_window(&mut shown, "https://behind.test/".to_owned(), false);
+        assert_eq!(again.as_deref(), Some("https://behind.test/"));
+    }
+
+    /// A link arriving while one is on screen and attended must not redress the
+    /// window: the user aimed at what they could see. And it must not be dropped
+    /// either — that click already happened.
     #[test]
     fn a_link_arriving_over_a_shown_one_waits_instead_of_replacing_it() {
         let mut shown = Shown::default();

crates/linkunbound-shell/ui/shell.slint

@@ -413,6 +413,7 @@ export component Picker inherits Window {
     title: "LinkUnbound";
     icon: @image-url("../../../app/src-tauri/icons/128x128.png");
     no-frame: true;
+    always-on-top: true;
     background: transparent;
     width: root.wanted-width;
     height: root.wanted-height;