From 25407f4e86062c080e6e7482dd7dbacf1a58a738 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 16 Aug 2023 10:16:31 -0500 Subject: [PATCH 1/3] First step toward not requiring text field nodes to have a value property; Windows only --- consumer/src/node.rs | 32 ++++++++++++++++++++++++++++---- consumer/src/text.rs | 8 ++++---- consumer/src/tree.rs | 1 + platforms/windows/src/adapter.rs | 2 +- platforms/windows/src/node.rs | 12 +++++++++--- 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/consumer/src/node.rs b/consumer/src/node.rs index 66a76efa..7f7fd8ac 100644 --- a/consumer/src/node.rs +++ b/consumer/src/node.rs @@ -50,6 +50,7 @@ impl<'a> Node<'a> { is_focused: self.is_focused(), is_root: self.is_root(), name: self.name(), + value: self.value(), live: self.live(), supports_text_ranges: self.supports_text_ranges(), } @@ -379,10 +380,6 @@ impl NodeState { self.data().checked_state() } - pub fn value(&self) -> Option<&str> { - self.data().value() - } - pub fn numeric_value(&self) -> Option { self.data().numeric_value() } @@ -534,6 +531,20 @@ impl<'a> Node<'a> { (!names.is_empty()).then(move || names.join(" ")) } } + + pub fn value(&self) -> Option { + if let Some(value) = &self.data().value() { + Some(value.to_string()) + } else if self.supports_text_ranges() && !self.is_multiline() { + Some(self.document_range().text()) + } else { + None + } + } + + pub fn has_value(&self) -> bool { + self.data().value().is_some() || (self.supports_text_ranges() && !self.is_multiline()) + } } impl NodeState { @@ -606,6 +617,10 @@ impl NodeState { pub fn raw_text_selection(&self) -> Option<&TextSelection> { self.data().text_selection() } + + pub fn raw_value(&self) -> Option<&str> { + self.data().value() + } } impl<'a> Node<'a> { @@ -680,6 +695,7 @@ pub struct DetachedNode { pub(crate) is_focused: bool, pub(crate) is_root: bool, pub(crate) name: Option, + pub(crate) value: Option, pub(crate) live: Live, pub(crate) supports_text_ranges: bool, } @@ -697,6 +713,14 @@ impl DetachedNode { self.name.clone() } + pub fn value(&self) -> Option { + self.value.clone() + } + + pub fn has_value(&self) -> bool { + self.value.is_some() + } + pub fn live(&self) -> Live { self.live } diff --git a/consumer/src/text.rs b/consumer/src/text.rs index d7dd5fac..ab6a97b0 100644 --- a/consumer/src/text.rs +++ b/consumer/src/text.rs @@ -60,7 +60,7 @@ impl<'a> InnerPosition<'a> { } fn is_paragraph_end(&self) -> bool { - self.is_line_end() && self.node.value().unwrap().ends_with('\n') + self.is_line_end() && self.node.data().value().unwrap().ends_with('\n') } fn is_document_start(&self, root_node: &Node) -> bool { @@ -237,7 +237,7 @@ impl<'a> Position<'a> { pub fn to_global_utf16_index(&self) -> usize { let mut total_length = 0usize; for node in self.root_node.inline_text_boxes() { - let node_text = node.value().unwrap(); + let node_text = node.data().value().unwrap(); if node.id() == self.inner.node.id() { let character_lengths = node.data().character_lengths(); let slice_end = character_lengths[..self.inner.character_index] @@ -538,7 +538,7 @@ impl<'a> Range<'a> { } else { character_lengths.len() }; - let value = node.value().unwrap(); + let value = node.data().value().unwrap(); let s = if start_index == end_index { "" } else if start_index == 0 && end_index == character_lengths.len() { @@ -983,7 +983,7 @@ impl<'a> Node<'a> { pub fn text_position_from_global_utf16_index(&self, index: usize) -> Option { let mut total_length = 0usize; for node in self.inline_text_boxes() { - let node_text = node.value().unwrap(); + let node_text = node.data().value().unwrap(); let node_text_length = node_text.chars().map(char::len_utf16).sum::(); let new_total_length = total_length + node_text_length; if index >= total_length && index < new_total_length { diff --git a/consumer/src/tree.rs b/consumer/src/tree.rs index cec6bd2e..6251aec9 100644 --- a/consumer/src/tree.rs +++ b/consumer/src/tree.rs @@ -182,6 +182,7 @@ impl State { is_focused: old_focus_id == Some(id), is_root: old_root_id == id, name: None, + value: None, live: Live::Off, supports_text_ranges: false, }; diff --git a/platforms/windows/src/adapter.rs b/platforms/windows/src/adapter.rs index f5fd9a3d..d32a182a 100644 --- a/platforms/windows/src/adapter.rs +++ b/platforms/windows/src/adapter.rs @@ -112,7 +112,7 @@ impl Adapter { } } fn node_updated(&mut self, old_node: &DetachedNode, new_node: &Node) { - if old_node.value() != new_node.value() { + if old_node.raw_value() != new_node.raw_value() { self.insert_text_change_if_needed(new_node); } if filter(new_node) != FilterResult::Include { diff --git a/platforms/windows/src/node.rs b/platforms/windows/src/node.rs index 0d4433c3..dd4fe8a8 100644 --- a/platforms/windows/src/node.rs +++ b/platforms/windows/src/node.rs @@ -358,15 +358,21 @@ impl<'a> NodeWrapper<'a> { } fn is_value_pattern_supported(&self) -> bool { - self.node_state().value().is_some() + match self { + Self::Node(node) => node.has_value(), + Self::DetachedNode(node) => node.has_value(), + } } fn is_range_value_pattern_supported(&self) -> bool { self.node_state().numeric_value().is_some() } - fn value(&self) -> &str { - self.node_state().value().unwrap() + fn value(&self) -> String { + match self { + Self::Node(node) => node.value().unwrap(), + Self::DetachedNode(node) => node.value().unwrap(), + } } fn is_read_only(&self) -> bool { From 8f9d57214ff1f78d96e2c8615f29c41bfc406327 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 16 Aug 2023 10:35:24 -0500 Subject: [PATCH 2/3] Fix build errors on macOS --- platforms/macos/src/node.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/platforms/macos/src/node.rs b/platforms/macos/src/node.rs index 566c43f3..bc443f7e 100644 --- a/platforms/macos/src/node.rs +++ b/platforms/macos/src/node.rs @@ -303,13 +303,20 @@ impl<'a> NodeWrapper<'a> { } } + fn node_value(&self) -> Option { + match self { + Self::Node(node) => node.value(), + Self::DetachedNode(node) => node.value(), + } + } + // TODO: implement proper logic for title, description, and value; // see Chromium's content/browser/accessibility/browser_accessibility_cocoa.mm // and figure out how this is different in the macOS 10.10+ protocol pub(crate) fn title(&self) -> Option { let state = self.node_state(); - if state.role() == Role::StaticText && state.value().is_none() { + if state.role() == Role::StaticText && state.raw_value().is_none() { // In this case, macOS wants the text to be the value, not title. return None; } @@ -321,8 +328,8 @@ impl<'a> NodeWrapper<'a> { if let Some(state) = state.checked_state() { return Some(Value::Bool(state != CheckedState::False)); } - if let Some(value) = state.value() { - return Some(Value::String(value.into())); + if let Some(value) = self.node_value() { + return Some(Value::String(value)); } if let Some(value) = state.numeric_value() { return Some(Value::Number(value)); From 06c1815259d3fa19829e25c4d4cbb8f0765e90f9 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Wed, 16 Aug 2023 11:18:30 -0500 Subject: [PATCH 3/3] Fire macOS value change events as needed for multi-line text controls where the provider doesn't directly set an outer value --- platforms/macos/src/event.rs | 59 +++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/platforms/macos/src/event.rs b/platforms/macos/src/event.rs index 51c1776b..64995dd1 100644 --- a/platforms/macos/src/event.rs +++ b/platforms/macos/src/event.rs @@ -3,13 +3,13 @@ // the LICENSE-APACHE file) or the MIT license (found in // the LICENSE-MIT file), at your option. -use accesskit::{Live, NodeId}; +use accesskit::{Live, NodeId, Role}; use accesskit_consumer::{DetachedNode, FilterResult, Node, TreeChangeHandler, TreeState}; use objc2::{ foundation::{NSInteger, NSMutableDictionary, NSNumber, NSObject, NSString}, msg_send, Message, }; -use std::rc::Rc; +use std::{collections::HashSet, rc::Rc}; use crate::{ appkit::*, @@ -132,6 +132,7 @@ impl QueuedEvents { pub(crate) struct EventGenerator { context: Rc, events: Vec, + text_changed: HashSet, } impl EventGenerator { @@ -139,6 +140,7 @@ impl EventGenerator { Self { context, events: Vec::new(), + text_changed: HashSet::new(), } } @@ -148,10 +150,56 @@ impl EventGenerator { events: self.events, } } + + fn insert_text_change_if_needed_parent(&mut self, node: Node) { + if !node.supports_text_ranges() { + return; + } + let id = node.id(); + if self.text_changed.contains(&id) { + return; + } + // Text change events must come before selection change + // events. It doesn't matter if text change events come + // before other events. + self.events.insert( + 0, + QueuedEvent::Generic { + node_id: id, + notification: unsafe { NSAccessibilityValueChangedNotification }, + }, + ); + self.text_changed.insert(id); + } + + fn insert_text_change_if_needed(&mut self, node: &Node) { + if node.role() != Role::InlineTextBox { + return; + } + if let Some(node) = node.filtered_parent(&filter) { + self.insert_text_change_if_needed_parent(node); + } + } + + fn insert_text_change_if_needed_for_removed_node( + &mut self, + node: &DetachedNode, + current_state: &TreeState, + ) { + if node.role() != Role::InlineTextBox { + return; + } + if let Some(id) = node.parent_id() { + if let Some(node) = current_state.node_by_id(id) { + self.insert_text_change_if_needed_parent(node); + } + } + } } impl TreeChangeHandler for EventGenerator { fn node_added(&mut self, node: &Node) { + self.insert_text_change_if_needed(node); if filter(node) != FilterResult::Include { return; } @@ -162,7 +210,9 @@ impl TreeChangeHandler for EventGenerator { } fn node_updated(&mut self, old_node: &DetachedNode, new_node: &Node) { - // TODO: text changes, live regions + if old_node.raw_value() != new_node.raw_value() { + self.insert_text_change_if_needed(new_node); + } if filter(new_node) != FilterResult::Include { return; } @@ -218,7 +268,8 @@ impl TreeChangeHandler for EventGenerator { } } - fn node_removed(&mut self, node: &DetachedNode, _current_state: &TreeState) { + fn node_removed(&mut self, node: &DetachedNode, current_state: &TreeState) { + self.insert_text_change_if_needed_for_removed_node(node, current_state); self.events.push(QueuedEvent::NodeDestroyed(node.id())); } }