diff --git a/clients/linux/src/main.rs b/clients/linux/src/main.rs index 7a90ff0c..dd7b5496 100644 --- a/clients/linux/src/main.rs +++ b/clients/linux/src/main.rs @@ -22,6 +22,9 @@ mod cli; mod shortcuts; #[cfg(target_os = "linux")] mod spawn; +// The guarded FlowBox `child-activated → activate` bridge every card grid needs. +#[cfg(target_os = "linux")] +mod ui_flow; #[cfg(target_os = "linux")] mod ui_hosts; #[cfg(target_os = "linux")] diff --git a/clients/linux/src/ui_flow.rs b/clients/linux/src/ui_flow.rs new file mode 100644 index 00000000..a8626b34 --- /dev/null +++ b/clients/linux/src/ui_flow.rs @@ -0,0 +1,70 @@ +//! One shared fix for a GTK4 footgun every card grid in this shell walks into. +//! +//! A pointer click (and keyboard activate) on a [`gtk::FlowBoxChild`] emits +//! `child-activated` on the *FlowBox*, never the child's own `activate` signal — so a +//! grid whose per-card handler hangs off `child.connect_activate()` has to bridge the +//! one to the other. The naive bridge is a stack overflow: `FlowBoxChild`'s default +//! `activate` handler re-emits `child-activated` on its parent, which calls the bridge, +//! which activates the child again, forever. +//! +//! [`bridge_child_activation`] is that bridge with the re-entrancy guard that breaks the +//! cycle. Use it for every `child-activated → child.activate()` hop; do not hand-roll it, +//! since a bare `flow.connect_child_activated(|_, c| c.activate())` aborts the process on +//! the first click and looks perfectly reasonable in review. + +use gtk::prelude::*; + +/// Bridge a FlowBox's `child-activated` to the activated child's own `activate` signal, +/// exactly once per click. The re-entrant emission the child's default handler bounces +/// back is swallowed rather than recursed into. +pub(crate) fn bridge_child_activation(flow: >k::FlowBox) { + let activating = std::cell::Cell::new(false); + flow.connect_child_activated(move |_, child| { + if activating.replace(true) { + return; + } + child.activate(); + activating.set(false); + }); +} + +#[cfg(test)] +mod tests { + use super::bridge_child_activation; + use gtk::prelude::*; + use std::cell::Cell; + use std::rc::Rc; + + // Reproduces the exact FlowBox/FlowBoxChild wiring the card grids use: the bridge + // calls `child.activate()`, whose own default handler re-emits `child-activated` — + // that ping-pong recursed forever (a real stack overflow on every card click/Enter, + // reported on the hosts page and then again on the library page) until the + // re-entrancy guard landed here, where both pages share it. + #[test] + #[ignore = "needs a Wayland/X display"] + fn flow_box_activation_bridge_does_not_recurse() { + assert!(gtk::init().is_ok(), "no display"); + + let flow = gtk::FlowBox::builder() + .selection_mode(gtk::SelectionMode::None) + .activate_on_single_click(true) + .build(); + bridge_child_activation(&flow); + + let child = gtk::FlowBoxChild::new(); + flow.insert(&child, -1); + let fired = Rc::new(Cell::new(0u32)); + { + let fired = fired.clone(); + child.connect_activate(move |_| fired.set(fired.get() + 1)); + } + + flow.emit_by_name::<()>("child-activated", &[&child]); + + assert_eq!( + fired.get(), + 1, + "the per-card handler should fire exactly once" + ); + } +} diff --git a/clients/linux/src/ui_hosts.rs b/clients/linux/src/ui_hosts.rs index 76e12576..28484bc0 100644 --- a/clients/linux/src/ui_hosts.rs +++ b/clients/linux/src/ui_hosts.rs @@ -781,18 +781,11 @@ impl SimpleComponent for HostsPage { // A pointer click (and keyboard activate) emits `child-activated` on the // *FlowBox*, never the child's own `activate` signal — bridge it back to the - // child, where each card wires its connect handler. The re-entrancy flag breaks - // the child-activated ↔ activate ping-pong that otherwise recurses forever - // (a real stack overflow on every card click; see the ignored display test). + // child, where each card wires its connect handler. The guard inside the bridge + // breaks the child-activated ↔ activate ping-pong that otherwise recurses forever + // (a real stack overflow on every card click; see `ui_flow`'s display test). for flow in [saved.widget(), discovered.widget()] { - let activating = std::cell::Cell::new(false); - flow.connect_child_activated(move |_, child| { - if activating.replace(true) { - return; - } - child.activate(); - activating.set(false); - }); + crate::ui_flow::bridge_child_activation(flow); } // Shown under the discovered heading while no (unsaved) advert is live yet. @@ -1466,49 +1459,3 @@ impl HostsPage { dialog.present(Some(&self.widgets.stack)); } } - -#[cfg(test)] -mod tests { - use adw::prelude::*; - use std::cell::Cell; - use std::rc::Rc; - - // Reproduces the exact FlowBox/FlowBoxChild wiring from `init()`: `child-activated` - // bridges to `child.activate()`, whose own default handler re-emits - // `child-activated` — that ping-pong recursed forever (stack overflow on every - // host-card click/Enter) until the re-entrancy guard was added. - #[test] - #[ignore = "needs a Wayland/X display"] - fn flow_box_activation_bridge_does_not_recurse() { - assert!(gtk::init().is_ok(), "no display"); - - let flow = gtk::FlowBox::builder() - .selection_mode(gtk::SelectionMode::None) - .activate_on_single_click(true) - .build(); - let activating = Cell::new(false); - flow.connect_child_activated(move |_, child| { - if activating.replace(true) { - return; - } - child.activate(); - activating.set(false); - }); - - let child = gtk::FlowBoxChild::new(); - flow.insert(&child, -1); - let fired = Rc::new(Cell::new(0u32)); - { - let fired = fired.clone(); - child.connect_activate(move |_| fired.set(fired.get() + 1)); - } - - flow.emit_by_name::<()>("child-activated", &[&child]); - - assert_eq!( - fired.get(), - 1, - "the per-card handler should fire exactly once" - ); - } -} diff --git a/clients/linux/src/ui_library.rs b/clients/linux/src/ui_library.rs index 7ec5d6d3..6574ab0b 100644 --- a/clients/linux/src/ui_library.rs +++ b/clients/linux/src/ui_library.rs @@ -116,10 +116,9 @@ fn build( .valign(gtk::Align::Start) .build(); // Click/keyboard activation fires `child-activated` on the FlowBox, not the child's own - // `activate` — bridge it so each poster's connect handler (below) runs on click. - flow.connect_child_activated(|_, child| { - child.activate(); - }); + // `activate` — bridge it so each poster's connect handler (below) runs on click. The + // bridge must be the guarded one: bare, it recurses until the stack overflows. + crate::ui_flow::bridge_child_activation(&flow); // The launcher shelf: same tile geometry as the games grid, its own FlowBox so the two // groups never interleave and each wraps on its own. let launcher_flow = gtk::FlowBox::builder() @@ -132,9 +131,7 @@ fn build( .row_spacing(18) .valign(gtk::Align::Start) .build(); - launcher_flow.connect_child_activated(|_, child| { - child.activate(); - }); + crate::ui_flow::bridge_child_activation(&launcher_flow); let launchers_heading = gtk::Label::new(Some("Launchers")); launchers_heading.add_css_class("pf-group-heading"); launchers_heading.set_halign(gtk::Align::Start);