Skip to content

Commit

Permalink
refactor!: Drop DefaultActionVerb (#472)
Browse files Browse the repository at this point in the history
  • Loading branch information
mwcampbell authored Oct 24, 2024
1 parent 2fa0d3f commit ef3b003
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 117 deletions.
58 changes: 8 additions & 50 deletions common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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))]
Expand Down Expand Up @@ -779,7 +743,6 @@ enum PropertyValue {
Invalid(Invalid),
Toggled(Toggled),
Live(Live),
DefaultActionVerb(DefaultActionVerb),
TextDirection(TextDirection),
Orientation(Orientation),
SortDirection(SortDirection),
Expand Down Expand Up @@ -892,7 +855,6 @@ enum PropertyId {
Invalid,
Toggled,
Live,
DefaultActionVerb,
TextDirection,
Orientation,
SortDirection,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -1957,7 +1918,6 @@ impl Serialize for Properties {
Invalid,
Toggled,
Live,
DefaultActionVerb,
TextDirection,
Orientation,
SortDirection,
Expand Down Expand Up @@ -2086,7 +2046,6 @@ impl<'de> Visitor<'de> for PropertiesVisitor {
Invalid { Invalid },
Toggled { Toggled },
Live { Live },
DefaultActionVerb { DefaultActionVerb },
TextDirection { TextDirection },
Orientation { Orientation },
SortDirection { SortDirection },
Expand Down Expand Up @@ -2234,7 +2193,6 @@ impl JsonSchema for Properties {
Invalid { Invalid },
Toggled { Toggled },
Live { Live },
DefaultActionVerb { DefaultActionVerb },
TextDirection { TextDirection },
Orientation { Orientation },
SortDirection { SortDirection },
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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()
);

Expand All @@ -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()
);

Expand Down
33 changes: 7 additions & 26 deletions consumer/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
29 changes: 9 additions & 20 deletions platforms/atspi-common/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
})?;
Expand Down
2 changes: 1 addition & 1 deletion platforms/macos/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
14 changes: 7 additions & 7 deletions platforms/windows/examples/hello_world.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}

Expand Down Expand Up @@ -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 _),
)
Expand Down Expand Up @@ -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) };
Expand Down
10 changes: 5 additions & 5 deletions platforms/windows/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, (
Expand Down Expand Up @@ -923,7 +923,7 @@ patterns! {
(IsSelected, is_selected, BOOL)
), (
fn Select(&self) -> Result<()> {
self.do_default_action()
self.click()
},

fn AddToSelection(&self) -> Result<()> {
Expand Down
Loading

0 comments on commit ef3b003

Please sign in to comment.