Skip to content
Open
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
98 changes: 78 additions & 20 deletions adapters/atspi-common/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,26 +617,28 @@ mod tests {
use super::Adapter;
use crate::{AdapterCallback, AppContext, CacheEvent, Event, InterfaceSet, WindowBounds};
use accesskit::{
ActionHandler, ActionRequest, Node, NodeId, Role, TreeId, TreeInfo, TreeUpdate,
Action, ActionData, ActionHandler, ActionRequest, Node, NodeId, Role, TreeId, TreeInfo,
TreeUpdate,
};
use accesskit_consumer::FullNodeId;
use atspi_common::Interface;
use std::sync::{Arc, Mutex};

type Log<T> = Arc<Mutex<Vec<T>>>;

#[derive(Clone, Copy, Debug, PartialEq)]
enum CacheOp {
Added(FullNodeId),
Removed(FullNodeId),
}

struct CapturingCallback {
ops: Arc<Mutex<Vec<CacheOp>>>,
}
struct Recorder<T>(Log<T>);

impl AdapterCallback for CapturingCallback {
impl AdapterCallback for Recorder<CacheOp> {
fn register_interfaces(&self, _: &Adapter, _: FullNodeId, _: InterfaceSet) {}
fn unregister_interfaces(&self, _: &Adapter, _: FullNodeId, _: InterfaceSet) {}
fn emit_event(&self, _: &Adapter, event: Event) {
let mut ops = self.ops.lock().unwrap();
let mut ops = self.0.lock().unwrap();
match event {
Event::Cache(CacheEvent::Added(id)) => ops.push(CacheOp::Added(id)),
Event::Cache(CacheEvent::Removed(id)) => ops.push(CacheOp::Removed(id)),
Expand All @@ -645,9 +647,10 @@ mod tests {
}
}

struct NoOpActionHandler;
impl ActionHandler for NoOpActionHandler {
fn do_action(&mut self, _request: ActionRequest) {}
impl ActionHandler for Recorder<ActionRequest> {
fn do_action(&mut self, request: ActionRequest) {
self.0.lock().unwrap().push(request);
}
}

fn with_children(role: Role, children: &[NodeId]) -> Node {
Expand All @@ -656,18 +659,19 @@ mod tests {
node
}

fn build(initial: TreeUpdate) -> (Adapter, Arc<Mutex<Vec<CacheOp>>>) {
fn build(initial: TreeUpdate) -> (Adapter, Log<CacheOp>, Log<ActionRequest>) {
let ops = Arc::new(Mutex::new(Vec::new()));
let actions = Arc::new(Mutex::new(Vec::new()));
let app_context = AppContext::new(None);
let adapter = Adapter::new(
&app_context,
CapturingCallback { ops: ops.clone() },
Recorder(ops.clone()),
initial,
false,
WindowBounds::default(),
NoOpActionHandler,
Recorder(actions.clone()),
);
(adapter, ops)
(adapter, ops, actions)
}

fn initial_tree() -> TreeUpdate {
Expand All @@ -691,15 +695,69 @@ mod tests {
}
}

#[test]
fn editable_text_support_and_dispatch() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests only cover the behavior of nodes so they should probably live in the node module. We may need to move some shared utilities in lib.rs.

let mut text_input = Node::new(Role::TextInput);
text_input.add_action(Action::SetValue);
let (mut adapter, _, requests) = build(TreeUpdate {
nodes: vec![(NodeId(0), text_input.clone())],
..initial_tree()
});
let platform = adapter.platform_node(adapter.root_id());
assert!(
platform
.interfaces()
.unwrap()
.contains(Interface::EditableText)
);

#[cfg(feature = "simplified-api")]
let node = crate::simplified::Accessible::Node(platform.clone());
#[cfg(not(feature = "simplified-api"))]
let node = platform.clone();

assert!(node.supports_editable_text().unwrap());
assert!(node.set_text_contents("hello").unwrap());

let mut read_only_input = text_input.clone();
read_only_input.set_read_only();
let mut numeric_input = text_input.clone();
numeric_input.set_numeric_value(0.0);
let mut button = Node::new(Role::Button);
button.add_action(Action::SetValue);
for unsupported in [
Node::new(Role::TextInput),
read_only_input,
numeric_input,
button,
] {
adapter.update(update(vec![(NodeId(0), unsupported)]));
assert!(!node.supports_editable_text().unwrap());
assert!(matches!(
node.set_text_contents("ignored"),
Err(crate::Error::UnsupportedInterface)
));
}
assert_eq!(
requests.lock().unwrap().as_slice(),
&[ActionRequest {
action: Action::SetValue,
target_tree: TreeId::ROOT,
target_node: NodeId(0),
data: Some(ActionData::Value("hello".into())),
}]
);
}

#[test]
fn no_cache_events_on_construction() {
let (_adapter, ops) = build(initial_tree());
let (_adapter, ops, _) = build(initial_tree());
assert!(ops.lock().unwrap().is_empty());
}

#[test]
fn add_node_emits_one_added() {
let (mut adapter, ops) = build(initial_tree());
let (mut adapter, ops, _) = build(initial_tree());
ops.lock().unwrap().clear();
adapter.update(update(vec![
(
Expand All @@ -715,7 +773,7 @@ mod tests {

#[test]
fn remove_node_emits_removed_for_same_id() {
let (mut adapter, ops) = build(initial_tree());
let (mut adapter, ops, _) = build(initial_tree());
adapter.update(update(vec![
(
NodeId(0),
Expand All @@ -737,7 +795,7 @@ mod tests {

#[test]
fn subtree_add_emits_added_per_node() {
let (mut adapter, ops) = build(initial_tree());
let (mut adapter, ops, _) = build(initial_tree());
ops.lock().unwrap().clear();
adapter.update(update(vec![
(
Expand All @@ -758,7 +816,7 @@ mod tests {

#[test]
fn subtree_remove_emits_removed_per_node() {
let (mut adapter, ops) = build(initial_tree());
let (mut adapter, ops, _) = build(initial_tree());
adapter.update(update(vec![
(
NodeId(0),
Expand All @@ -785,7 +843,7 @@ mod tests {
fn filter_transition_into_tree_emits_added() {
let mut hidden = Node::new(Role::Button);
hidden.set_hidden();
let (mut adapter, ops) = build(TreeUpdate {
let (mut adapter, ops, _) = build(TreeUpdate {
nodes: vec![
(
NodeId(0),
Expand All @@ -807,7 +865,7 @@ mod tests {

#[test]
fn filter_transition_out_of_tree_emits_removed() {
let (mut adapter, ops) = build(initial_tree());
let (mut adapter, ops, _) = build(initial_tree());
ops.lock().unwrap().clear();
let mut hidden = Node::new(Role::Button);
hidden.set_hidden();
Expand Down
93 changes: 56 additions & 37 deletions adapters/atspi-common/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
// found in the LICENSE.chromium file.

use accesskit::{
Action, ActionData, ActionRequest, Affine, Live, NodeId, Orientation, Point, Rect, Role,
Toggled, TreeId,
Action, ActionData, ActionRequest, Affine, Live, Orientation, Point, Rect, Role, Toggled,
};
use accesskit_consumer::{FilterResult, FullNodeId, NodeRef, Tree, TreeState};
use atspi_common::{
Expand Down Expand Up @@ -438,6 +437,15 @@ impl NodeWrapper<'_> {
self.0.raw_bounds().is_some() || self.is_root()
}

fn supports_editable_text(&self) -> bool {
// Empty inputs may have no text ranges. Numeric controls use AT-SPI Value because

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is just wrong, please remove it.

// SetValue doesn't declare which ActionData variant the handler accepts.
self.0.is_text_input()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other adapters the condition here should just be: self.is_text_input() && self.supports_text_ranges()

&& !self.0.is_read_only()
&& !self.supports_value()
&& self.0.supports_action(Action::SetValue, &filter)
}

fn supports_hyperlink(&self) -> bool {
self.0.supports_url()
}
Expand All @@ -462,6 +470,9 @@ impl NodeWrapper<'_> {
if self.supports_component() {
interfaces.insert(Interface::Component);
}
if self.supports_editable_text() {
interfaces.insert(Interface::EditableText);
}
if self.supports_hyperlink() {
interfaces.insert(Interface::Hyperlink);
}
Expand Down Expand Up @@ -759,20 +770,32 @@ impl PlatformNode {
self.resolve_for_text_with_context(|node, _, _| f(node))
}

fn do_action_internal<F>(&self, target: FullNodeId, f: F) -> Result<()>
where
F: FnOnce(&TreeState, &Context, NodeId, TreeId) -> ActionRequest,
{
fn dispatch_action(&self, action: Action, data: Option<ActionData>) -> Result<()> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already know that a node implements the interface because the D-Bus object exists, no need to check it again here. You could check for the read-only property in PlatformNode::set_text_contents but I'd get rid of this entire refactoring. Don't worry too much about the simplified API.

self.dispatch_checked_action(action, data, |_| true)
}

fn dispatch_checked_action(
&self,
action: Action,
data: Option<ActionData>,
supports: impl for<'a> FnOnce(NodeRef<'a>) -> bool,
) -> Result<()> {
let context = self.upgrade_context()?;
let tree = context.read_tree();
if let Some((target_node, target_tree)) = tree.state().locate_node(target) {
let request = f(tree.state(), &context, target_node, target_tree);
drop(tree);
context.do_action(request);
Ok(())
} else {
Err(Error::Defunct)
let state = tree.state();
let node = state.node_by_id(self.id).ok_or(Error::Defunct)?;
if !supports(node) {
return Err(Error::UnsupportedInterface);
}
let (target_node, target_tree) = state.locate_node(self.id).ok_or(Error::Defunct)?;
drop(tree);
context.do_action(ActionRequest {
action,
target_tree,
target_node,
data,
});
Ok(())
}

pub fn name(&self) -> Result<String> {
Expand Down Expand Up @@ -966,6 +989,10 @@ impl PlatformNode {
})
}

pub fn supports_editable_text(&self) -> Result<bool> {
self.resolve(|node| Ok(NodeWrapper(&node).supports_editable_text()))
}

pub fn supports_hyperlink(&self) -> Result<bool> {
self.resolve(|node| {
let wrapper = NodeWrapper(&node);
Expand Down Expand Up @@ -1035,12 +1062,7 @@ impl PlatformNode {
if index != 0 {
return Ok(false);
}
self.do_action_internal(self.id, |_, _, target_node, target_tree| ActionRequest {
action: Action::Click,
target_tree,
target_node,
data: None,
})?;
self.dispatch_action(Action::Click, None)?;
Ok(true)
}

Expand Down Expand Up @@ -1096,22 +1118,15 @@ impl PlatformNode {
}

pub fn grab_focus(&self) -> Result<bool> {
self.do_action_internal(self.id, |_, _, target_node, target_tree| ActionRequest {
action: Action::Focus,
target_tree,
target_node,
data: None,
})?;
self.dispatch_action(Action::Focus, None)?;
Ok(true)
}

pub fn scroll_to(&self, scroll_type: ScrollType) -> Result<bool> {
self.do_action_internal(self.id, |_, _, target_node, target_tree| ActionRequest {
action: Action::ScrollIntoView,
target_tree,
target_node,
data: atspi_scroll_type_to_scroll_hint(scroll_type).map(ActionData::ScrollHint),
})?;
self.dispatch_action(
Action::ScrollIntoView,
atspi_scroll_type_to_scroll_hint(scroll_type).map(ActionData::ScrollHint),
)?;
Ok(true)
}

Expand Down Expand Up @@ -1656,12 +1671,16 @@ impl PlatformNode {
}

pub fn set_current_value(&self, value: f64) -> Result<()> {
self.do_action_internal(self.id, |_, _, target_node, target_tree| ActionRequest {
action: Action::SetValue,
target_tree,
target_node,
data: Some(ActionData::NumericValue(value)),
})
self.dispatch_action(Action::SetValue, Some(ActionData::NumericValue(value)))
}

pub fn set_text_contents(&self, value: &str) -> Result<bool> {
self.dispatch_checked_action(
Action::SetValue,
Some(ActionData::Value(value.into())),
|node| NodeWrapper(&node).supports_editable_text(),
)?;
Ok(true)
}
}

Expand Down
14 changes: 14 additions & 0 deletions adapters/atspi-common/src/simplified.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,20 @@ impl Accessible {
}
}

pub fn supports_editable_text(&self) -> Result<bool> {
match self {
Self::Node(node) => node.supports_editable_text(),
Self::Root(_) => Ok(false),
}
}

pub fn set_text_contents(&self, value: &str) -> Result<bool> {
match self {
Self::Node(node) => node.set_text_contents(value),
Self::Root(_) => Err(Error::UnsupportedInterface),
}
}

pub fn supports_hyperlink(&self) -> Result<bool> {
match self {
Self::Node(node) => node.supports_hyperlink(),
Expand Down
8 changes: 8 additions & 0 deletions adapters/unix/src/atspi/bus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ impl Bus {
)
.await?;
}
if new_interfaces.contains(Interface::EditableText) {
self.register_interface(&path, EditableTextInterface::new(node.clone()))
.await?;
}
if new_interfaces.contains(Interface::Hyperlink) {
self.register_interface(
&path,
Expand Down Expand Up @@ -200,6 +204,10 @@ impl Bus {
self.unregister_interface::<ComponentInterface>(&path)
.await?;
}
if old_interfaces.contains(Interface::EditableText) {
self.unregister_interface::<EditableTextInterface>(&path)
.await?;
}
if old_interfaces.contains(Interface::Hyperlink) {
self.unregister_interface::<HyperlinkInterface>(&path)
.await?;
Expand Down
Loading