Basic AccessKit support for exit confirmation dialog

[DataTriny]

Part of #1904

Start exposing niri's UI to assistive technologies. This is gated behind the more generic accessibility feature (enabled by default and tied to the dbus feature since the AT-SPI protocol uses it). No bounds information areprovided for now because it still isn't clear how to do this under Wayland.

This can be tested by starting the Orca screen reader inside a niri session and then pressing Super+Shift+E. Orca will announce the alert and its content. One big issue though is that these dialogs seem to not steal the focus to other apps, so Orca won't notify the user if they are dismissed. Orca users expect to hear which widget had the focus in the app before the modal dialog showed up.

[YaLTeR]

Thanks, very interesting!

This is gated behind the more generic accessibility feature (enabled by default and tied to the dbus feature since the AT-SPI protocol uses it).

Why do you think this is necessary? Is there some nontrivial overhead, or does it not build on musl or FreeBSD?

One big issue though is that these dialogs seem to not steal the focus to other apps, so Orca won't notify the user if they are dismissed. Orca users expect to hear which widget had the focus in the app before the modal dialog showed up.

I see, that makes sense. I can change it to have a proper focus (might be a bit involved with the current code; don't remember off the bat).

It's more problematic design-wise for the Important Hotkeys dialog: my intention was to make it as unobstructive as possible, hence the current behavior where any input hides it and goes "through" immediately. Not sure what's the best way to deal with it. Perhaps make it have proper focus, but only when a screen reader is attached? Is that something we know?

[YaLTeR]

I guess this should ideally be long-lived on the full Niri state struct, as the global accessibility provider object?

[DataTriny]

Roughly speaking, one AccessKit unix adapter equals one top-level window. It sends events to assistive technologies letting them know that the window was closed when the adapter is dropped. When we'll have a main background window, it will make sense to store its adapter in a more central state.

[YaLTeR]

And I guess that this should be true only when some niri UI has keyboard focus, rather than a window or a layer surface?

[DataTriny]

If I understand correctly, this is not an issue here because the focus should immediately be put on this dialog when it is open, and there is no way for something else to steal it since any action which is not pressing the Enter key will close the dialog. Am I right?

[YaLTeR]

That is correct; I'm thinking more about the future general case that handles several niri UI elements, rather than code specific to this one dialog.

[DataTriny]

Why do you think this is necessary? Is there some nontrivial overhead, or does it not build on musl or FreeBSD?

Due to how AT-SPI works, it brings non trivial overhead but only when an assistive technology is actually running. Do you think we should simply use the dbus feature instead?

Perhaps make it have proper focus, but only when a screen reader is attached? Is that something we know?

ActivationHandler::initial_tree_requested could be used to enable a global flag somewhere since it will only be called if an assistive technology is running. Similarely, the deactivation handler could be used to do the oppositve.

But I'd suggest avoiding special paths for assistive technology users if possible, as this is more stuff to test. The reason why I didn't include the config error UI here is because it is clear to me that it is a notification and that it will require a different accessibility tree. But the exit confirmation dialog clearly is modal since it forces the user to interact with it by pressing either Enter to accept or anything else to dismiss.

[YaLTeR]

Due to how AT-SPI works, it brings non trivial overhead but only when an assistive technology is actually running. Do you think we should simply use the dbus feature instead?

I think that's fine. Anyone who doesn't need a11y won't enable anything from what I understand, and anyone who needs a11y probably doesn't want to find out that their compositor has it compiled out.

ActivationHandler::initial_tree_requested could be used to enable a global flag somewhere since it will only be called if an assistive technology is running. Similarely, the deactivation handler could be used to do the oppositve.

I see, sounds good.

But I'd suggest avoiding special paths for assistive technology users if possible, as this is more stuff to test.

That is a fair point yeah, I'll consider it when this is more in shape.

The reason why I didn't include the config error UI here is because it is clear to me that it is a notification and that it will require a different accessibility tree.

What would be good documentation on how these different UI components should be represented in the accessibility tree? I'm guessing web ARIA stuff is documented well somewhere, but it probably doesn't include a lot of desktop-specific concepts like notifications?

[DataTriny]

I think that's fine. Anyone who doesn't need a11y won't enable anything from what I understand, and anyone who needs a11y probably doesn't want to find out that their compositor has it compiled out

Totally agree, I'll move everything to the dbus feature then. If taking away the keyboard focus isn't too much work, I can add it in here as well (just for the exit confirmation dialog I mean).

What would be good documentation on how these different UI components should be represented in the accessibility tree?

Documentation is something we still need to work on with AccessKit. Right now I can't tell you how to properly implement notification. We'll have to figure it out, and niri can be a good test bed.

[YaLTeR]

If taking away the keyboard focus isn't too much work, I can add it in here as well (just for the exit confirmation dialog I mean).

I suspect it might be a bit involved. We don't have a UI toolkit in niri, so these things are kind of hand-coded in places. So it's better if I look into it (I don't think it should be difficult, just don't recall off the bat what exactly to change).

Documentation is something we still need to work on with AccessKit. Right now I can't tell you how to properly implement notification. We'll have to figure it out, and niri can be a good test bed.

I see, alright!

[YaLTeR]

Do I understand correctly that the reason why we need to take away keyboard focus from a window is because giving it focus back (after dismissing the dialog) is what causes Orca to re-announce the widget?

[YaLTeR]

Try this patch for the focus:

diff --git i/src/handlers/xdg_shell.rs w/src/handlers/xdg_shell.rs
index a9eceb67..d10bd106 100644
--- i/src/handlers/xdg_shell.rs
+++ w/src/handlers/xdg_shell.rs
@@ -303,7 +303,16 @@ impl XdgShellHandler for State {
         // We need to hand out the grab in a way consistent with what update_keyboard_focus()
         // thinks the current focus is, otherwise it will desync and cause weird issues with
         // keyboard focus being at the wrong place.
-        if self.niri.is_locked() {
+        if self
+            .niri
+            .exit_confirm_dialog
+            .as_ref()
+            .is_some_and(|d| d.is_open())
+        {
+            trace!("ignoring popup grab because the exit confirm dialog is open");
+            let _ = PopupManager::dismiss_popup(&root, &popup);
+            return;
+        } else if self.niri.is_locked() {
             if Some(&root) != self.niri.lock_surface_focus().as_ref() {
                 trace!("ignoring popup grab because the session is locked");
                 let _ = PopupManager::dismiss_popup(&root, &popup);
diff --git i/src/niri.rs w/src/niri.rs
index 1d82ba6e..17596de5 100644
--- i/src/niri.rs
+++ w/src/niri.rs
@@ -509,6 +509,7 @@ pub enum KeyboardFocus {
     LayerShell { surface: WlSurface },
     LockScreen { surface: Option<WlSurface> },
     ScreenshotUi,
+    ExitConfirmDialog,
     Overview,
 }
 
@@ -606,6 +607,7 @@ impl KeyboardFocus {
             KeyboardFocus::LayerShell { surface } => Some(surface),
             KeyboardFocus::LockScreen { surface } => surface.as_ref(),
             KeyboardFocus::ScreenshotUi => None,
+            KeyboardFocus::ExitConfirmDialog => None,
             KeyboardFocus::Overview => None,
         }
     }
@@ -616,6 +618,7 @@ impl KeyboardFocus {
             KeyboardFocus::LayerShell { surface } => Some(surface),
             KeyboardFocus::LockScreen { surface } => surface,
             KeyboardFocus::ScreenshotUi => None,
+            KeyboardFocus::ExitConfirmDialog => None,
             KeyboardFocus::Overview => None,
         }
     }
@@ -1058,7 +1061,14 @@ impl State {
         }
 
         // Compute the current focus.
-        let focus = if self.niri.is_locked() {
+        let focus = if self
+            .niri
+            .exit_confirm_dialog
+            .as_ref()
+            .is_some_and(|d| d.is_open())
+        {
+            KeyboardFocus::ExitConfirmDialog
+        } else if self.niri.is_locked() {
             KeyboardFocus::LockScreen {
                 surface: self.niri.lock_surface_focus(),
             }
@@ -3918,6 +3928,7 @@ impl Niri {
             // layer-shell, the layout will briefly draw as active, despite never having focus.
             KeyboardFocus::LockScreen { .. } => true,
             KeyboardFocus::ScreenshotUi => true,
+            KeyboardFocus::ExitConfirmDialog => true,
             KeyboardFocus::Overview => true,
         };

One behavioral difference from this is that now the exit confirm dialog will close popups and some layer-shell launchers (they close upon losing focus), but I guess it's fine.