HitFrostbite/punktfunk
1
0
Fork 0
forked from unom/punktfunk
Code Pull Requests Activity

Merge pull request 'The library grid never got the FlowBox recursion guard, so every game click overflowed the stack' (#264) from worktree-gtk-library-card-activation-overflow into main

Browse Source 
Reviewed-on: unom/punktfunk#264
This commit is contained in:
enricobuehler
2026-08-16 08:46:55 +00:00
parent fb9f605120 9e71cda298
commit 65b4af9e51
4 changed files with 81 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
clients/linux/src/main.rs
+3
View File
@@ -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")]
clients/linux/src/ui_flow.rs
+70
View File
@@ -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: &gtk::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"
);
}
}
clients/linux/src/ui_hosts.rs
+4 -57
View File
@@ -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"
);
}
}
clients/linux/src/ui_library.rs
+4 -7
View File
@@ -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);