refactor!: Drop DefaultActionVerb (#472)

mwcampbell · web-flow · commit ef3b00382244 · 2024-10-24T23:21:19.000+02:00
diff --git a/common/src/lib.rs b/common/src/lib.rs
@@ -268,9 +268,6 @@ pub enum Role {
 }
 
 /// An action to be taken on an accessibility node.
-///
-/// In contrast to [`DefaultActionVerb`], these describe what happens to the
-/// object, e.g. "focus".
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "enumn", derive(enumn::N))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
@@ -282,8 +279,8 @@ pub enum Role {
 )]
 #[repr(u8)]
 pub enum Action {
-    /// Do the default action for an object, typically this means "click".
-    Default,
+    /// Do the equivalent of a single click or tap.
+    Click,
 
     Focus,
     Blur,
@@ -357,7 +354,7 @@ impl Action {
         // want to bring this crate by default though and we can't use a
         // macro as it would break C bindings header file generation.
         match value {
-            0 => Some(Action::Default),
+            0 => Some(Action::Click),
             1 => Some(Action::Focus),
             2 => Some(Action::Blur),
             3 => Some(Action::Collapse),
@@ -468,39 +465,6 @@ pub enum Toggled {
     Mixed,
 }
 
-/// Describes the action that will be performed on a given node when
-/// executing the default action, which is a click.
-///
-/// In contrast to [`Action`], these describe what the user can do on the
-/// object, e.g. "press", not what happens to the object as a result.
-/// Only one verb can be used at a time to describe the default action.
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "enumn", derive(enumn::N))]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[cfg_attr(feature = "schemars", derive(JsonSchema))]
-#[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
-#[cfg_attr(
-    feature = "pyo3",
-    pyclass(module = "accesskit", rename_all = "SCREAMING_SNAKE_CASE")
-)]
-#[repr(u8)]
-pub enum DefaultActionVerb {
-    Click,
-    Focus,
-    Check,
-    Uncheck,
-    /// A click will be performed on one of the node's ancestors.
-    /// This happens when the node itself is not clickable, but one of its
-    /// ancestors has click handlers attached which are able to capture the click
-    /// as it bubbles up.
-    ClickAncestor,
-    Jump,
-    Open,
-    Press,
-    Select,
-    Unselect,
-}
-
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "enumn", derive(enumn::N))]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
@@ -779,7 +743,6 @@ enum PropertyValue {
     Invalid(Invalid),
     Toggled(Toggled),
     Live(Live),
-    DefaultActionVerb(DefaultActionVerb),
     TextDirection(TextDirection),
     Orientation(Orientation),
     SortDirection(SortDirection),
@@ -892,7 +855,6 @@ enum PropertyId {
     Invalid,
     Toggled,
     Live,
-    DefaultActionVerb,
     TextDirection,
     Orientation,
     SortDirection,
@@ -1775,7 +1737,6 @@ unique_enum_property_methods! {
     (Invalid, invalid, set_invalid, clear_invalid),
     (Toggled, toggled, set_toggled, clear_toggled),
     (Live, live, set_live, clear_live),
-    (DefaultActionVerb, default_action_verb, set_default_action_verb, clear_default_action_verb),
     (TextDirection, text_direction, set_text_direction, clear_text_direction),
     (Orientation, orientation, set_orientation, clear_orientation),
     (SortDirection, sort_direction, set_sort_direction, clear_sort_direction),
@@ -1957,7 +1918,6 @@ impl Serialize for Properties {
                 Invalid,
                 Toggled,
                 Live,
-                DefaultActionVerb,
                 TextDirection,
                 Orientation,
                 SortDirection,
@@ -2086,7 +2046,6 @@ impl<'de> Visitor<'de> for PropertiesVisitor {
                 Invalid { Invalid },
                 Toggled { Toggled },
                 Live { Live },
-                DefaultActionVerb { DefaultActionVerb },
                 TextDirection { TextDirection },
                 Orientation { Orientation },
                 SortDirection { SortDirection },
@@ -2234,7 +2193,6 @@ impl JsonSchema for Properties {
             Invalid { Invalid },
             Toggled { Toggled },
             Live { Live },
-            DefaultActionVerb { DefaultActionVerb },
             TextDirection { TextDirection },
             Orientation { Orientation },
             SortDirection { SortDirection },
@@ -2437,7 +2395,7 @@ mod tests {
 
     #[test]
     fn action_n() {
-        assert_eq!(Action::n(0), Some(Action::Default));
+        assert_eq!(Action::n(0), Some(Action::Click));
         assert_eq!(Action::n(1), Some(Action::Focus));
         assert_eq!(Action::n(2), Some(Action::Blur));
         assert_eq!(Action::n(3), Some(Action::Collapse));
@@ -2475,9 +2433,9 @@ mod tests {
         );
 
         let mut builder = NodeBuilder::new(Role::Unknown);
-        builder.add_action(Action::Default);
+        builder.add_action(Action::Click);
         assert_eq!(
-            &[Action::Default],
+            &[Action::Click],
             action_mask_to_action_vec(builder.actions).as_slice()
         );
 
@@ -2489,10 +2447,10 @@ mod tests {
         );
 
         let mut builder = NodeBuilder::new(Role::Unknown);
-        builder.add_action(Action::Default);
+        builder.add_action(Action::Click);
         builder.add_action(Action::ShowContextMenu);
         assert_eq!(
-            &[Action::Default, Action::ShowContextMenu],
+            &[Action::Click, Action::ShowContextMenu],
             action_mask_to_action_vec(builder.actions).as_slice()
         );
 
diff --git a/consumer/src/node.rs b/consumer/src/node.rs
@@ -11,8 +11,8 @@
 use std::{iter::FusedIterator, sync::Arc};
 
 use accesskit::{
-    Action, Affine, DefaultActionVerb, Live, Node as NodeData, NodeId, Orientation, Point, Rect,
-    Role, TextSelection, Toggled,
+    Action, Affine, Live, Node as NodeData, NodeId, Orientation, Point, Rect, Role, TextSelection,
+    Toggled,
 };
 
 use crate::filters::FilterResult;
@@ -404,31 +404,17 @@ impl<'a> Node<'a> {
         self.data().orientation()
     }
 
-    pub fn default_action_verb(&self) -> Option<DefaultActionVerb> {
-        self.data().default_action_verb()
-    }
-
     // When probing for supported actions as the next several functions do,
     // it's tempting to check the role. But it's better to not assume anything
     // beyond what the provider has explicitly told us. Rationale:
-    // if the provider developer forgot to correctly set `default_action_verb`,
+    // if the provider developer forgot to call `add_action` for an action,
     // an AT (or even AccessKit itself) can fall back to simulating
     // a mouse click. But if the provider doesn't handle an action request
     // and we assume that it will based on the role, the attempted action
     // does nothing. This stance is a departure from Chromium.
 
     pub fn is_clickable(&self) -> bool {
-        // If it has a custom default action verb except for
-        // `DefaultActionVerb::ClickAncestor`, it's definitely clickable.
-        // `DefaultActionVerb::ClickAncestor` is used when a node with a
-        // click listener is present in its ancestry chain.
-        if let Some(verb) = self.default_action_verb() {
-            if verb != DefaultActionVerb::ClickAncestor {
-                return true;
-            }
-        }
-
-        false
+        self.supports_action(Action::Click)
     }
 
     pub fn supports_toggle(&self) -> bool {
@@ -449,16 +435,11 @@ impl<'a> Node<'a> {
         // "invocable", as the "invoke" action would be a redundant synonym
         // for the "set focus" action. The same logic applies to selection.
         self.is_clickable()
-            && !matches!(
-                self.default_action_verb(),
-                Some(
-                    DefaultActionVerb::Focus
-                        | DefaultActionVerb::Select
-                        | DefaultActionVerb::Unselect
-                )
-            )
+            && !self.is_text_input()
+            && !matches!(self.role(), Role::Document | Role::Terminal)
             && !self.supports_toggle()
             && !self.supports_expand_collapse()
+            && self.is_selected().is_none()
     }
 
     // The future of the `Action` enum is undecided, so keep the following
diff --git a/platforms/atspi-common/src/node.rs b/platforms/atspi-common/src/node.rs
@@ -9,8 +9,8 @@
 // found in the LICENSE.chromium file.
 
 use accesskit::{
-    Action, ActionData, ActionRequest, Affine, DefaultActionVerb, Live, NodeId, Orientation, Point,
-    Rect, Role, Toggled,
+    Action, ActionData, ActionRequest, Affine, Live, NodeId, Orientation, Point, Rect, Role,
+    Toggled,
 };
 use accesskit_consumer::{FilterResult, Node, TreeState};
 use atspi_common::{
@@ -361,7 +361,7 @@ impl<'a> NodeWrapper<'a> {
     }
 
     fn supports_action(&self) -> bool {
-        self.0.default_action_verb().is_some()
+        self.0.is_clickable()
     }
 
     fn supports_component(&self) -> bool {
@@ -403,29 +403,18 @@ impl<'a> NodeWrapper<'a> {
     }
 
     fn n_actions(&self) -> i32 {
-        match self.0.default_action_verb() {
-            Some(_) => 1,
-            None => 0,
+        if self.0.is_clickable() {
+            1
+        } else {
+            0
         }
     }
 
     fn get_action_name(&self, index: i32) -> String {
         if index != 0 {
             return String::new();
         }
-        String::from(match self.0.default_action_verb() {
-            Some(DefaultActionVerb::Click) => "click",
-            Some(DefaultActionVerb::Focus) => "focus",
-            Some(DefaultActionVerb::Check) => "check",
-            Some(DefaultActionVerb::Uncheck) => "uncheck",
-            Some(DefaultActionVerb::ClickAncestor) => "clickAncestor",
-            Some(DefaultActionVerb::Jump) => "jump",
-            Some(DefaultActionVerb::Open) => "open",
-            Some(DefaultActionVerb::Press) => "press",
-            Some(DefaultActionVerb::Select) => "select",
-            Some(DefaultActionVerb::Unselect) => "unselect",
-            None => "",
-        })
+        String::from(if self.0.is_clickable() { "click" } else { "" })
     }
 
     fn raw_bounds_and_transform(&self) -> (Option<Rect>, Affine) {
@@ -870,7 +859,7 @@ impl PlatformNode {
             return Ok(false);
         }
         self.do_action_internal(|_, _| ActionRequest {
-            action: Action::Default,
+            action: Action::Click,
             target: self.id,
             data: None,
         })?;
diff --git a/platforms/macos/src/node.rs b/platforms/macos/src/node.rs
@@ -553,7 +553,7 @@ declare_class!(
                 let clickable = node.is_clickable();
                 if clickable {
                     context.do_action(ActionRequest {
-                        action: Action::Default,
+                        action: Action::Click,
                         target: node.id(),
                         data: None,
                     });
diff --git a/platforms/windows/examples/hello_world.rs b/platforms/windows/examples/hello_world.rs
@@ -1,8 +1,8 @@
 // Based on the create_window sample in windows-samples-rs.
 
 use accesskit::{
-    Action, ActionHandler, ActionRequest, ActivationHandler, DefaultActionVerb, Live, Node,
-    NodeBuilder, NodeId, Rect, Role, Tree, TreeUpdate,
+    Action, ActionHandler, ActionRequest, ActivationHandler, Live, Node, NodeBuilder, NodeId, Rect,
+    Role, Tree, TreeUpdate,
 };
 use accesskit_windows::Adapter;
 use once_cell::sync::Lazy;
@@ -59,7 +59,7 @@ const BUTTON_2_RECT: Rect = Rect {
 };
 
 const SET_FOCUS_MSG: u32 = WM_USER;
-const DO_DEFAULT_ACTION_MSG: u32 = WM_USER + 1;
+const CLICK_MSG: u32 = WM_USER + 1;
 
 fn build_button(id: NodeId, name: &str) -> Node {
     let rect = match id {
@@ -72,7 +72,7 @@ fn build_button(id: NodeId, name: &str) -> Node {
     builder.set_bounds(rect);
     builder.set_name(name);
     builder.add_action(Action::Focus);
-    builder.set_default_action_verb(DefaultActionVerb::Click);
+    builder.add_action(Action::Click);
     builder.build()
 }
 
@@ -206,11 +206,11 @@ impl ActionHandler for SimpleActionHandler {
                 }
                 .unwrap();
             }
-            Action::Default => {
+            Action::Click => {
                 unsafe {
                     PostMessageW(
                         self.window,
-                        DO_DEFAULT_ACTION_MSG,
+                        CLICK_MSG,
                         WPARAM(0),
                         LPARAM(request.target.0 as _),
                     )
@@ -308,7 +308,7 @@ extern "system" fn wndproc(window: HWND, message: u32, wparam: WPARAM, lparam: L
             }
             LRESULT(0)
         }
-        DO_DEFAULT_ACTION_MSG => {
+        CLICK_MSG => {
             let id = NodeId(lparam.0 as _);
             if id == BUTTON_1_ID || id == BUTTON_2_ID {
                 let state = unsafe { &*get_window_state(window) };
diff --git a/platforms/windows/src/node.rs b/platforms/windows/src/node.rs
@@ -597,8 +597,8 @@ impl PlatformNode {
         Ok(())
     }
 
-    fn do_default_action(&self) -> Result<()> {
-        self.do_action(|| (Action::Default, None))
+    fn click(&self) -> Result<()> {
+        self.do_action(|| (Action::Click, None))
     }
 
     fn relative(&self, node_id: NodeId) -> Self {
@@ -886,12 +886,12 @@ patterns! {
         (ToggleState, toggle_state, ToggleState)
     ), (
         fn Toggle(&self) -> Result<()> {
-            self.do_default_action()
+            self.click()
         }
     )),
     (Invoke, is_invoke_pattern_supported, (), (
         fn Invoke(&self) -> Result<()> {
-            self.do_default_action()
+            self.click()
         }
     )),
     (Value, is_value_pattern_supported, (
@@ -923,7 +923,7 @@ patterns! {
         (IsSelected, is_selected, BOOL)
     ), (
         fn Select(&self) -> Result<()> {
-            self.do_default_action()
+            self.click()
         },
 
         fn AddToSelection(&self) -> Result<()> {
diff --git a/platforms/winit/examples/mixed_handlers.rs b/platforms/winit/examples/mixed_handlers.rs
diff --git a/platforms/winit/examples/simple.rs b/platforms/winit/examples/simple.rs
