Activate newly mapped windows with a valid activation token

most of the time the activation token is passed
while the window is still unmapped. in this case
store the intend to activate the window for
later retrieval on map.

niri-visual-tests/src/cases/layout.rs

@@ -3,7 +3,7 @@ use std::time::Duration;
 
 use niri::animation::Clock;
 use niri::layout::workspace::ColumnWidth;
-use niri::layout::{LayoutElement as _, Options};
+use niri::layout::{ActivateWindow, LayoutElement as _, Options};
 use niri::render_helpers::RenderTarget;
 use niri_config::{Color, FloatOrInt, OutputName};
 use smithay::backend::renderer::element::RenderElement;
@@ -162,7 +162,8 @@ impl Layout {
         window.request_size(ws.new_window_size(width, window.rules()), false, None);
         window.communicate();
 
-        self.layout.add_window(window.clone(), width, false);
+        self.layout
+            .add_window(window.clone(), width, false, ActivateWindow::default());
         self.windows.push(window);
     }
 

src/handlers/compositor.rs

@@ -18,6 +18,8 @@ use smithay::wayland::shm::{ShmHandler, ShmState};
 use smithay::{delegate_compositor, delegate_shm};
 
 use super::xdg_shell::add_mapped_toplevel_pre_commit_hook;
+use crate::handlers::XDG_ACTIVATION_TOKEN_TIMEOUT;
+use crate::layout::ActivateWindow;
 use crate::niri::{ClientState, State};
 use crate::utils::send_scale_transform;
 use crate::utils::transaction::Transaction;
@@ -84,7 +86,11 @@ impl CompositorHandler for State {
 
                 if is_mapped {
                     // The toplevel got mapped.
-                    let Unmapped { window, state } = entry.remove();
+                    let Unmapped {
+                        window,
+                        state,
+                        activation_token_data,
+                    } = entry.remove();
 
                     window.on_commit();
 
@@ -133,8 +139,20 @@ impl CompositorHandler for State {
                     let mapped = Mapped::new(window, rules, hook);
                     let window = mapped.window.clone();
 
+                    // Check the token timestamp again in case the window took a while between
+                    // requesting activation and mapping.
+                    let activate = match activation_token_data
+                        .filter(|token| token.timestamp.elapsed() < XDG_ACTIVATION_TOKEN_TIMEOUT)
+                    {
+                        Some(_) => ActivateWindow::Yes,
+                        None => ActivateWindow::Smart,
+                    };
+
                     let output = if let Some(p) = parent {
                         // Open dialogs immediately to the right of their parent window.
+                        //
+                        // FIXME: do we want to use activate here? How do we want things to behave
+                        // exactly?
                         self.niri
                             .layout
                             .add_window_right_of(&p, mapped, width, is_full_width)
@@ -144,14 +162,21 @@ impl CompositorHandler for State {
                             mapped,
                             width,
                             is_full_width,
+                            activate,
                         )
                     } else if let Some(output) = &output {
-                        self.niri
+                        self.niri.layout.add_window_on_output(
-                            .layout
+                            output,
-                            .add_window_on_output(output, mapped, width, is_full_width);
+                            mapped,
+                            width,
+                            is_full_width,
+                            activate,
+                        );
                         Some(output)
                     } else {
-                        self.niri.layout.add_window(mapped, width, is_full_width)
+                        self.niri
+                            .layout
+                            .add_window(mapped, width, is_full_width, activate)
                     };
 
                     if let Some(output) = output.cloned() {

src/handlers/mod.rs

@@ -670,10 +670,12 @@ impl XdgActivationHandler for State {
                 self.niri.layout.activate_window(&window);
                 self.niri.layer_shell_on_demand_focus = None;
                 self.niri.queue_redraw_all();
-
+            } else if let Some(unmapped) = self.niri.unmapped_windows.get_mut(&surface) {
-                self.niri.activation_state.remove_token(&token);
+                unmapped.activation_token_data = Some(token_data);
             }
         }
+
+        self.niri.activation_state.remove_token(&token);
     }
 }
 delegate_xdg_activation!(State);

src/handlers/xdg_shell.rs

@@ -752,7 +752,7 @@ impl State {
             self.niri.is_at_startup,
         );
 
-        let Unmapped { window, state } = unmapped;
+        let Unmapped { window, state, .. } = unmapped;
 
         let InitialConfigureState::NotConfigured { wants_fullscreen } = state else {
             error!("window must not be already configured in send_initial_configure()");

src/layout/mod.rs

@@ -358,6 +358,18 @@ pub struct RemovedTile<W: LayoutElement> {
     is_full_width: bool,
 }
 
+/// Whether to activate a newly added window.
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+pub enum ActivateWindow {
+    /// Activate unconditionally.
+    Yes,
+    /// Activate based on heuristics.
+    #[default]
+    Smart,
+    /// Do not activate.
+    No,
+}
+
 impl<W: LayoutElement> InteractiveMoveState<W> {
     fn moving(&self) -> Option<&InteractiveMoveData<W>> {
         match self {
@@ -383,6 +395,16 @@ impl<W: LayoutElement> InteractiveMoveData<W> {
     }
 }
 
+impl ActivateWindow {
+    pub fn map_smart(self, f: impl FnOnce() -> bool) -> bool {
+        match self {
+            ActivateWindow::Yes => true,
+            ActivateWindow::Smart => f(),
+            ActivateWindow::No => false,
+        }
+    }
+}
+
 impl Options {
     fn from_config(config: &Config) -> Self {
         let layout = &config.layout;
@@ -753,6 +775,7 @@ impl<W: LayoutElement> Layout<W> {
         window: W,
         width: Option<ColumnWidth>,
         is_full_width: bool,
+        activate: ActivateWindow,
     ) -> Option<&Output> {
         let width = self.resolve_default_width(&window, width);
 
@@ -771,20 +794,27 @@ impl<W: LayoutElement> Layout<W> {
                     })
                     .unwrap();
 
-                // Don't steal focus from an active fullscreen window.
+                if activate == ActivateWindow::Yes {
-                let mut activate = true;
+                    *active_monitor_idx = mon_idx;
-                let ws = &mon.workspaces[ws_idx];
-                if mon_idx == *active_monitor_idx
-                    && !ws.columns.is_empty()
-                    && ws.columns[ws.active_column_idx].is_fullscreen
-                {
-                    activate = false;
                 }
 
-                // Don't activate if on a different workspace.
+                let activate = activate.map_smart(|| {
-                if mon.active_workspace_idx != ws_idx {
+                    // Don't steal focus from an active fullscreen window.
-                    activate = false;
+                    let ws = &mon.workspaces[ws_idx];
-                }
+                    if mon_idx == *active_monitor_idx
+                        && !ws.columns.is_empty()
+                        && ws.columns[ws.active_column_idx].is_fullscreen
+                    {
+                        return false;
+                    }
+
+                    // Don't activate if on a different workspace.
+                    if mon.active_workspace_idx != ws_idx {
+                        return false;
+                    }
+
+                    true
+                });
 
                 mon.add_window(ws_idx, window, activate, width, is_full_width);
                 Some(&mon.output)
@@ -798,7 +828,8 @@ impl<W: LayoutElement> Layout<W> {
                             .map_or(false, |name| name.eq_ignore_ascii_case(workspace_name))
                     })
                     .unwrap();
-                ws.add_window(None, window, true, width, is_full_width);
+                let activate = activate.map_smart(|| true);
+                ws.add_window(None, window, activate, width, is_full_width);
                 None
             }
         }
@@ -835,6 +866,7 @@ impl<W: LayoutElement> Layout<W> {
         window: W,
         width: Option<ColumnWidth>,
         is_full_width: bool,
+        activate: ActivateWindow,
     ) -> Option<&Output> {
         let width = self.resolve_default_width(&window, width);
 
@@ -846,12 +878,11 @@ impl<W: LayoutElement> Layout<W> {
             } => {
                 let mon = &mut monitors[*active_monitor_idx];
 
-                // Don't steal focus from an active fullscreen window.
+                let activate = activate.map_smart(|| {
-                let mut activate = true;
+                    // Don't steal focus from an active fullscreen window.
-                let ws = &mon.workspaces[mon.active_workspace_idx];
+                    let ws = &mon.workspaces[mon.active_workspace_idx];
-                if !ws.columns.is_empty() && ws.columns[ws.active_column_idx].is_fullscreen {
+                    ws.columns.is_empty() || !ws.columns[ws.active_column_idx].is_fullscreen
-                    activate = false;
+                });
-                }
 
                 mon.add_window(
                     mon.active_workspace_idx,
@@ -872,7 +903,8 @@ impl<W: LayoutElement> Layout<W> {
                     ));
                     &mut workspaces[0]
                 };
-                ws.add_window(None, window, true, width, is_full_width);
+                let activate = activate.map_smart(|| true);
+                ws.add_window(None, window, activate, width, is_full_width);
                 None
             }
         }
@@ -893,11 +925,12 @@ impl<W: LayoutElement> Layout<W> {
         if let Some(InteractiveMoveState::Moving(move_)) = &self.interactive_move {
             if right_of == move_.tile.window().id() {
                 let output = move_.output.clone();
+                let activate = ActivateWindow::default();
                 if self.monitor_for_output(&output).is_some() {
-                    self.add_window_on_output(&output, window, width, is_full_width);
+                    self.add_window_on_output(&output, window, width, is_full_width, activate);
                     return Some(&self.monitor_for_output(&output).unwrap().output);
                 } else {
-                    return self.add_window(window, width, is_full_width);
+                    return self.add_window(window, width, is_full_width, activate);
                 }
             }
         }
@@ -932,6 +965,7 @@ impl<W: LayoutElement> Layout<W> {
         window: W,
         width: Option<ColumnWidth>,
         is_full_width: bool,
+        activate: ActivateWindow,
     ) {
         let width = self.resolve_default_width(&window, width);
 
@@ -950,16 +984,22 @@ impl<W: LayoutElement> Layout<W> {
             .find(|(_, mon)| mon.output == *output)
             .unwrap();
 
-        // Don't steal focus from an active fullscreen window.
+        if activate == ActivateWindow::Yes {
-        let mut activate = true;
+            *active_monitor_idx = mon_idx;
-        let ws = &mon.workspaces[mon.active_workspace_idx];
-        if mon_idx == *active_monitor_idx
-            && !ws.columns.is_empty()
-            && ws.columns[ws.active_column_idx].is_fullscreen
-        {
-            activate = false;
         }
 
+        let activate = activate.map_smart(|| {
+            // Don't steal focus from an active fullscreen window.
+            let ws = &mon.workspaces[mon.active_workspace_idx];
+            if mon_idx == *active_monitor_idx
+                && !ws.columns.is_empty()
+                && ws.columns[ws.active_column_idx].is_fullscreen
+            {
+                return false;
+            }
+            true
+        });
+
         mon.add_window(
             mon.active_workspace_idx,
             window,
@@ -4365,7 +4405,7 @@ mod tests {
                     }
 
                     let win = TestWindow::new(id, bbox, min_max_size.0, min_max_size.1);
-                    layout.add_window(win, None, false);
+                    layout.add_window(win, None, false, ActivateWindow::default());
                 }
                 Op::AddWindowRightOf {
                     id,
@@ -4482,7 +4522,13 @@ mod tests {
                     }
 
                     let win = TestWindow::new(id, bbox, min_max_size.0, min_max_size.1);
-                    layout.add_window_to_named_workspace(&ws_name, win, None, false);
+                    layout.add_window_to_named_workspace(
+                        &ws_name,
+                        win,
+                        None,
+                        false,
+                        ActivateWindow::default(),
+                    );
                 }
                 Op::CloseWindow(id) => {
                     layout.remove_window(&id, Transaction::new());

src/window/unmapped.rs

@@ -1,6 +1,7 @@
 use smithay::desktop::Window;
 use smithay::output::Output;
 use smithay::wayland::shell::xdg::ToplevelSurface;
+use smithay::wayland::xdg_activation::XdgActivationTokenData;
 
 use super::ResolvedWindowRules;
 use crate::layout::workspace::ColumnWidth;
@@ -9,6 +10,8 @@ use crate::layout::workspace::ColumnWidth;
 pub struct Unmapped {
     pub window: Window,
     pub state: InitialConfigureState,
+    /// Activation token, if one was used on this unmapped window.
+    pub activation_token_data: Option<XdgActivationTokenData>,
 }
 
 #[allow(clippy::large_enum_variant)]
@@ -57,6 +60,7 @@ impl Unmapped {
             state: InitialConfigureState::NotConfigured {
                 wants_fullscreen: None,
             },
+            activation_token_data: None,
         }
     }