Detect WM_HINTS popup (yabridge popups fix) (#328)

* Detect WM_HINTS popup (yabridge popups fix)

Many windows popups have:

WM_HINTS(WM_HINTS) Client accepts input or input focus: False

Which is a good indicator window SHOULD be a popup.

This is true for example on yabridge plugins and some other apps.
However toplevel windows have this property True. In order to
differentiate them we check if there are no decorations on the window
(client) and this property is False.
Its applied ONLY to _NET_WM_WINDOW_TYPE_NORMAL windows since its most
generic one that comes up.

This was tested across many apps: Reaper, Ardour, Godot, MaterialMaker,
PixelOver, PixelComposer, Steam, Steam games (both windowed and
fullscreen), Fusion 9, Unity and others I have.

There are no regression seen in any of tested apps and fixes yabridge
popups.
Additionally check if MOTIF has functions that should not be in popup
Combine wmhint popup check with skip_taskbar.
wmhint is only considered to be a popup if application has skip_taskbar
atom.
Fixed edgecases so far:
BattleNet spawning as popupp
PixelComposer spawning as popup

src/server/tests.rs

@@ -1325,6 +1325,7 @@ fn window_group_properties() {
         win,
         super::WmHints {
             window_group: Some(prop_win),
+            acquire_input_via_wm: false,
         },
     );
     f.satellite.map_window(win);

src/xstate/mod.rs

@@ -626,6 +626,7 @@ impl XState {
         let class = self.get_wm_class(window);
         let size_hints = self.get_wm_size_hints(window);
         let motif_wm_hints = self.get_motif_wm_hints(window);
+        let wm_hints = self.get_wm_hints(window);
         let mut title = name.resolve()?;
         if title.is_none() {
             title = self.get_wm_name(window).resolve()?;
@@ -640,7 +641,7 @@ impl XState {
         if let Some(hints) = size_hints.resolve()? {
             server_state.set_size_hints(window, hints);
         }
-
+        let wmhints = wm_hints.resolve()?;
         let motif_hints = motif_wm_hints.resolve()?;
         if let Some(decorations) = motif_hints.as_ref().and_then(|m| m.decorations) {
             server_state.set_win_decorations(window, decorations);
@@ -657,7 +658,8 @@ impl XState {
             .resolve()?
             .flatten();
 
-        let is_popup = self.guess_is_popup(window, motif_hints, transient_for.is_some())?;
+        let is_popup =
+            self.guess_is_popup(window, motif_hints, wmhints, transient_for.is_some())?;
         server_state.set_popup(window, is_popup);
         if let Some(parent) = transient_for.and_then(|t| (!is_popup).then_some(t)) {
             server_state.set_transient_for(window, parent);
@@ -685,18 +687,12 @@ impl XState {
         &self,
         window: x::Window,
         motif_hints: Option<motif::Hints>,
+        wm_hints: Option<WmHints>,
         has_transient_for: bool,
     ) -> XResult<bool> {
         let mut motif_popup = false;
-        if let Some(hints) = motif_hints {
+        let mut wmhint_popup = false;
-            // If MOTIF_WM_HINTS provides no decorations for client assume its a popup
+        let mut has_skip_taskbar = false;
-            motif_popup = hints.decorations.is_some_and(|d| d.is_clientside());
-            // If the motif hints indicate the user shouldn't be able to do anything
-            // to the window at all, it stands to reason it's probably a popup.
-            if hints.functions.is_some_and(|f| f.is_empty()) {
-                return Ok(true);
-            }
-        }
 
         let attrs = self
             .connection
@@ -713,6 +709,33 @@ impl XState {
             atoms_vec,
         );
 
+        if let Some(states) = window_state.resolve()? {
+            has_skip_taskbar = states.contains(&self.atoms.skip_taskbar);
+        }
+        if let Some(hints) = motif_hints {
+            // If MOTIF_WM_HINTS provides no decorations for client assume its a popup
+            motif_popup = hints.decorations.is_some_and(|d| d.is_clientside());
+            // WMHINTS is considered popup only if client is not decorated && client does not
+            // accept input focus
+            // Sometimes popup is false-positive meaning both MOTIF Decorations and WM_HINTS input indicates its a popup
+            // but MOTIF has function flags that toplevel window should do
+            // Also combine wmhint_popup with skip_taskbar which
+            // fixes some edge cases where certain apps (BattleNet client, PixelComposer spawn as popup)
+            wmhint_popup = motif_popup
+                && wm_hints.is_some_and(|h| !h.acquire_input_via_wm)
+                && !hints.functions.as_ref().is_some_and(|f| {
+                    f.contains(motif::Functions::Minimize)
+                        || f.contains(motif::Functions::Maximize)
+                        || f.contains(motif::Functions::All)
+                })
+                && has_skip_taskbar;
+            // If the motif hints indicate the user shouldn't be able to do anything
+            // to the window at all, it stands to reason it's probably a popup.
+            if hints.functions.is_some_and(|f| f.is_empty()) {
+                return Ok(true);
+            }
+        }
+
         let override_redirect = self.connection.wait_for_reply(attrs)?.override_redirect();
         let mut is_popup = override_redirect;
 
@@ -738,9 +761,8 @@ impl XState {
         let mut known_window_type = false;
         for ty in window_types {
             match ty {
-                x if x == self.window_atoms.normal || x == self.window_atoms.dialog => {
+                x if x == self.window_atoms.normal => is_popup = override_redirect || wmhint_popup,
-                    is_popup = override_redirect
+                x if x == self.window_atoms.dialog => is_popup = override_redirect,
-                }
                 x if x == self.window_atoms.utility => {
                     is_popup = override_redirect || motif_popup;
                 }
@@ -765,9 +787,7 @@ impl XState {
         }
 
         if !known_window_type {
-            if let Some(states) = window_state.resolve()? {
+            is_popup = has_skip_taskbar;
-                is_popup = states.contains(&self.atoms.skip_taskbar);
-            }
         }
 
         Ok(is_popup)
@@ -1073,6 +1093,7 @@ bitflags! {
 bitflags! {
     /// https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.2.4
     pub struct WmHintsFlags: u32 {
+        const Input = 1;
         const WindowGroup = 64;
     }
 }
@@ -1115,6 +1136,7 @@ impl From<&[u32]> for WmNormalHints {
 #[derive(Default, Debug, PartialEq, Eq)]
 pub struct WmHints {
     pub window_group: Option<x::Window>,
+    pub acquire_input_via_wm: bool,
 }
 
 impl From<&[u32]> for WmHints {
@@ -1126,6 +1148,9 @@ impl From<&[u32]> for WmHints {
             let window = x::Window::new(value[8]);
             ret.window_group = Some(window);
         }
+        if flags.contains(WmHintsFlags::Input) {
+            ret.acquire_input_via_wm = value[1] == 1;
+        }
 
         ret
     }

tests/integration.rs

@@ -346,6 +346,7 @@ xcb::atoms_struct! {
         win_type_utility => b"_NET_WM_WINDOW_TYPE_UTILITY",
         win_type_dnd => b"_NET_WM_WINDOW_TYPE_DND",
         motif_wm_hints => b"_MOTIF_WM_HINTS" only_if_exists = false,
+        wm_hints => b"WM_HINTS",
         mime1 => b"text/plain" only_if_exists = false,
         mime2 => b"blah/blah" only_if_exists = false,
         incr => b"INCR",
@@ -2071,6 +2072,21 @@ fn popup_heuristics() {
     );
     f.map_as_popup(&mut connection, godot_popup);
 
+    let material_maker_popup = connection.new_window(connection.root, 10, 10, 50, 50, false);
+    connection.set_property(
+        material_maker_popup,
+        x::ATOM_ATOM,
+        connection.atoms.win_type,
+        &[connection.atoms.win_type_utility],
+    );
+    connection.set_property(
+        material_maker_popup,
+        connection.atoms.motif_wm_hints,
+        connection.atoms.motif_wm_hints,
+        &[0x2_u32, 0, 0, 0, 0],
+    );
+    f.map_as_popup(&mut connection, material_maker_popup);
+
     let ardour_toplevel = connection.new_window(connection.root, 10, 10, 50, 50, false);
     connection.set_property(
         ardour_toplevel,
@@ -2079,6 +2095,75 @@ fn popup_heuristics() {
         &[connection.atoms.win_type_utility],
     );
     f.map_as_toplevel(&mut connection, ardour_toplevel);
+
+    let yabridge_popup = connection.new_window(connection.root, 10, 10, 50, 50, false);
+    connection.set_property(
+        yabridge_popup,
+        x::ATOM_ATOM,
+        connection.atoms.win_type,
+        &[connection.atoms.win_type_normal],
+    );
+    connection.set_property(
+        yabridge_popup,
+        connection.atoms.motif_wm_hints,
+        connection.atoms.motif_wm_hints,
+        &[0x2_u32, 0, 0, 0, 0],
+    );
+    connection.set_property(
+        yabridge_popup,
+        connection.atoms.wm_hints,
+        connection.atoms.wm_hints,
+        &[0x1_u32, 0, 0, 0, 0, 0, 0, 0, 0],
+    );
+    connection.set_property(
+        yabridge_popup,
+        x::ATOM_ATOM,
+        connection.atoms.net_wm_state,
+        &[connection.atoms.skip_taskbar],
+    );
+    f.map_as_popup(&mut connection, yabridge_popup);
+
+    let steam = connection.new_window(connection.root, 10, 10, 50, 50, false);
+    connection.set_property(
+        steam,
+        x::ATOM_ATOM,
+        connection.atoms.win_type,
+        &[connection.atoms.win_type_normal],
+    );
+    connection.set_property(
+        steam,
+        connection.atoms.motif_wm_hints,
+        connection.atoms.motif_wm_hints,
+        &[0x2_u32, 0, 0, 0, 0],
+    );
+    connection.set_property(
+        steam,
+        connection.atoms.wm_hints,
+        connection.atoms.wm_hints,
+        &[0x1_u32, 1, 0, 0, 0, 0, 0, 0, 0],
+    );
+    f.map_as_toplevel(&mut connection, steam);
+
+    let battle_net = connection.new_window(connection.root, 10, 10, 50, 50, false);
+    connection.set_property(
+        battle_net,
+        x::ATOM_ATOM,
+        connection.atoms.win_type,
+        &[connection.atoms.win_type_normal],
+    );
+    connection.set_property(
+        battle_net,
+        connection.atoms.motif_wm_hints,
+        connection.atoms.motif_wm_hints,
+        &[0x3_u32, 0x2c, 0x0, 0x0, 0x0],
+    );
+    connection.set_property(
+        battle_net,
+        connection.atoms.wm_hints,
+        connection.atoms.wm_hints,
+        &[0x1_u32, 0, 0, 0, 0, 0, 0, 0, 0],
+    );
+    f.map_as_toplevel(&mut connection, battle_net);
 }
 
 #[test]