From 3f2ac4d85c4e66c2f1ae6622bf3c2f302abe93b9 Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Fri, 30 May 2025 01:12:00 +0200 Subject: Detect XWayland dialogs via WM_TRANSIENT_FOR, not just native xdg_toplevel parent Window::is_dialog (close-button-only titlebar, no traffic lights) was only ever set from a native xdg_toplevel's own parent() - redraw_ decoration_buffer's is_dialog computation called dw.toplevel(), which is always None for an XWayland-backed DWindow (X11Surface's own accessor is x11_surface(), a different method), so the .unwrap_or(false) fallback made every XWayland dialog - a GTK "Save As", an app's own "About" box, anything setting the ICCCM transient-for hint - always draw with the full three-button titlebar and traffic-light colours, even though the feature this was built for explicitly wanted the opposite. Documented as a known gap at the time; now closed. redraw_decoration_buffer now also checks X11Surface::is_transient_for() for an XWayland window. property_notify gained a WmWindowProperty:: TransientFor arm that re-runs redraw_decoration_buffer, for a client that sets the hint slightly after its own initial map - the same "read fresh every call" pattern the existing xdg_toplevel::parent() check already relied on, extended to catch a late X11 property the way the Wayland equivalent (set_parent, any time) already was. --- crates/core/src/window.rs | 17 ++++++++--------- crates/wayland/src/state/lifecycle.rs | 23 ++++++++++++++++++----- crates/wayland/src/xwayland.rs | 15 +++++++++++++++ 3 files changed, 41 insertions(+), 14 deletions(-) (limited to 'crates') diff --git a/crates/core/src/window.rs b/crates/core/src/window.rs index b30cf7b..6e40486 100644 --- a/crates/core/src/window.rs +++ b/crates/core/src/window.rs @@ -160,15 +160,14 @@ pub struct Window { /// Geometry to restore to when un-maximizing. pub restore_geometry: Option, pub decorated: bool, - /// Whether this window declared an `xdg_toplevel` parent (`set_parent`) - /// - a dialog/utility window belonging to another one, not a normal - /// top-level app window. Backend-set (the wayland crate reads the real - /// `ToplevelSurface::parent()`, refreshed on every decoration redraw), - /// same as `decorated` itself; `core` has no protocol concept of its - /// own to derive this from. Only ever `true` for a genuine `xdg_ - /// toplevel` client that set a parent - an XWayland dialog's own - /// `WM_TRANSIENT_FOR` isn't read yet, so this misses those specifically - /// (a real, known gap, not an oversight). Requested directly: a + /// Whether this window declared itself a dialog/utility window + /// belonging to another one, not a normal top-level app window -- + /// a native `xdg_toplevel`'s own `parent()` (`set_parent`), or an + /// XWayland `X11Surface`'s ICCCM `WM_TRANSIENT_FOR` hint + /// (`is_transient_for()`). Backend-set (the wayland crate reads + /// whichever real accessor applies, refreshed on every decoration + /// redraw), same as `decorated` itself; `core` has no protocol + /// concept of its own to derive this from. Requested directly: a /// dialog's titlebar should show only a close button, no traffic /// lights - see `hit_test`'s and `decoration::render_titlebar`'s own /// use of this for what actually changes. diff --git a/crates/wayland/src/state/lifecycle.rs b/crates/wayland/src/state/lifecycle.rs index b47cb23..53a5e86 100644 --- a/crates/wayland/src/state/lifecycle.rs +++ b/crates/wayland/src/state/lifecycle.rs @@ -81,11 +81,24 @@ impl CompState { // change. Written back onto the real `Window` (not just used // locally) so `ResizeEdge::hit_test`'s own `is_dialog` parameter // - read from `core`, which has no protocol concept to derive - // this from itself - agrees with whatever got drawn here. An - // XWayland window's own `WM_TRANSIENT_FOR` isn't read yet, so this - // stays `false` for those specifically - see `Window::is_dialog`'s - // own doc comment. - let is_dialog = self.id_to_window.get(&id).and_then(|dw| dw.toplevel()).map(|t| t.parent().is_some()).unwrap_or(false); + // this from itself - agrees with whatever got drawn here. + // + // Checks both real toplevel kinds a `DWindow` can wrap: a native + // `xdg_toplevel`'s own `parent()`, or an XWayland `X11Surface`'s + // `WM_TRANSIENT_FOR` via `is_transient_for()`. The X11 half used + // to be unchecked entirely (`.toplevel()` alone, which is always + // `None` for an X11-backed window - `X11Surface`'s own accessor + // is `.x11_surface()`, a different method), so every XWayland + // dialog - a GTK "Save As", an app's own "About" box, anything + // that sets the ICCCM transient-for hint - always drew with the + // full three-button titlebar and traffic-light colours, the + // native-Wayland-only case this whole feature was built for. + // Reported live: "dialog windows... should never have traffic + // light, should just be x" - true for native Wayland dialogs + // already, not for XWayland ones. + let is_dialog = self.id_to_window.get(&id).is_some_and(|dw| { + dw.toplevel().is_some_and(|t| t.parent().is_some()) || dw.x11_surface().is_some_and(|x| x.is_transient_for().is_some()) + }); if let Some(win) = self.wm.borrow_mut().window_mut(id) { win.is_dialog = is_dialog; } diff --git a/crates/wayland/src/xwayland.rs b/crates/wayland/src/xwayland.rs index a51ee97..7834f16 100644 --- a/crates/wayland/src/xwayland.rs +++ b/crates/wayland/src/xwayland.rs @@ -825,6 +825,21 @@ impl XwmHandler for CompState { /// (a dock's running-indicator, an app switcher, icon lookup), not just /// this compositor's own UI. fn property_notify(&mut self, _xwm: XwmId, window: X11Surface, property: WmWindowProperty) { + // `WM_TRANSIENT_FOR` - `Window::is_dialog`'s own X11 half (see its + // doc comment) - can arrive after the window's already mapped and + // decorated: a client that sets it slightly late, or one this + // compositor granted the map request for before XWayland finished + // resolving the property. `redraw_decoration_buffer` re-reads + // `is_transient_for()` fresh every call, so simply calling it again + // here picks up the change - same "cheap once nothing's actually + // different" self-guard (`decoration_signatures`) every other + // redraw trigger in this codebase already relies on. + if matches!(property, WmWindowProperty::TransientFor) { + if let Some(&id) = self.xwayland_windows.get(&window.window_id()) { + self.redraw_decoration_buffer(id); + } + return; + } if !matches!(property, WmWindowProperty::Title | WmWindowProperty::Class) { return; } -- cgit v1.2.3