Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: Drop DefaultActionVerb #472

Merged
merged 1 commit into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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