feat!: Implement refactored ScrollIntoView action across desktop platforms (#594)

Author: mwcampbell

common/src/lib.rs

@@ -313,9 +313,8 @@ pub enum Action {
     /// Scroll up by the specified unit.
     ScrollUp,
 
-    /// Scroll any scrollable containers to make the target object visible
-    /// on the screen.  Optionally set [`ActionRequest::data`] to
-    /// [`ActionData::ScrollTargetRect`].
+    /// Scroll any scrollable containers to make the target node visible.
+    /// Optionally set [`ActionRequest::data`] to [`ActionData::ScrollHint`].
     ScrollIntoView,
 
     /// Scroll the given object to a specified point in the tree's container
@@ -2658,6 +2657,26 @@ pub enum ScrollUnit {
     Page,
 }
 
+/// A suggestion about where the node being scrolled into view should be
+/// positioned relative to the edges of the scrollable container.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[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", eq)
+)]
+#[repr(u8)]
+pub enum ScrollHint {
+    TopLeft,
+    BottomRight,
+    TopEdge,
+    BottomEdge,
+    LeftEdge,
+    RightEdge,
+}
+
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "schemars", derive(JsonSchema))]
@@ -2668,9 +2687,10 @@ pub enum ActionData {
     Value(Box<str>),
     NumericValue(f64),
     ScrollUnit(ScrollUnit),
-    /// Optional target rectangle for [`Action::ScrollIntoView`], in
-    /// the coordinate space of the action's target node.
-    ScrollTargetRect(Rect),
+    /// Optional suggestion for [`ActionData::ScrollIntoView`], specifying
+    /// the preferred position of the target node relative to the scrollable
+    /// container's viewport.
+    ScrollHint(ScrollHint),
     /// Target for [`Action::ScrollToPoint`], in platform-native coordinates
     /// relative to the origin of the tree's container (e.g. window).
     ScrollToPoint(Point),

platforms/atspi-common/src/node.rs

@@ -1022,6 +1022,15 @@ impl PlatformNode {
         Ok(true)
     }
 
+    pub fn scroll_to(&self, scroll_type: ScrollType) -> Result<bool> {
+        self.do_action_internal(|_, _| ActionRequest {
+            action: Action::ScrollIntoView,
+            target: self.id,
+            data: atspi_scroll_type_to_scroll_hint(scroll_type).map(ActionData::ScrollHint),
+        })?;
+        Ok(true)
+    }
+
     pub fn scroll_to_point(&self, coord_type: CoordType, x: i32, y: i32) -> Result<bool> {
         self.resolve_with_context(|node, context| {
             let window_bounds = context.read_root_window_bounds();
@@ -1378,14 +1387,22 @@ impl PlatformNode {
         &self,
         start_offset: i32,
         end_offset: i32,
-        _: ScrollType,
+        scroll_type: ScrollType,
     ) -> Result<bool> {
         self.resolve_for_text_with_context(|node, context| {
-            if let Some(rect) = text_range_bounds_from_offsets(&node, start_offset, end_offset) {
+            if let Some(range) = text_range_from_offsets(&node, start_offset, end_offset) {
+                let position = if matches!(
+                    scroll_type,
+                    ScrollType::BottomRight | ScrollType::BottomEdge | ScrollType::RightEdge
+                ) {
+                    range.end()
+                } else {
+                    range.start()
+                };
                 context.do_action(ActionRequest {
                     action: Action::ScrollIntoView,
-                    target: node.id(),
-                    data: Some(ActionData::ScrollTargetRect(rect)),
+                    target: position.inner_node().id(),
+                    data: atspi_scroll_type_to_scroll_hint(scroll_type).map(ActionData::ScrollHint),
                 });
                 Ok(true)
             } else {

platforms/atspi-common/src/simplified.rs

@@ -224,6 +224,13 @@ impl Accessible {
         }
     }
 
+    pub fn scroll_to(&self, scroll_type: ScrollType) -> Result<bool> {
+        match self {
+            Self::Node(node) => node.scroll_to(scroll_type),
+            Self::Root(_) => Err(Error::UnsupportedInterface),
+        }
+    }
+
     pub fn scroll_to_point(&self, coord_type: CoordType, x: i32, y: i32) -> Result<bool> {
         match self {
             Self::Node(node) => node.scroll_to_point(coord_type, x, y),

platforms/atspi-common/src/util.rs

@@ -3,9 +3,9 @@
 // the LICENSE-APACHE file) or the MIT license (found in
 // the LICENSE-MIT file), at your option.
 
-use accesskit::{Point, Rect};
+use accesskit::{Point, Rect, ScrollHint};
 use accesskit_consumer::{Node, TextPosition, TextRange};
-use atspi_common::{CoordType, Granularity};
+use atspi_common::{CoordType, Granularity, ScrollType};
 
 use crate::Error;
 
@@ -120,3 +120,15 @@ pub(crate) fn text_range_bounds_from_offsets(
         .into_iter()
         .reduce(|rect1, rect2| rect1.union(rect2))
 }
+
+pub(crate) fn atspi_scroll_type_to_scroll_hint(scroll_type: ScrollType) -> Option<ScrollHint> {
+    match scroll_type {
+        ScrollType::TopLeft => Some(ScrollHint::TopLeft),
+        ScrollType::BottomRight => Some(ScrollHint::BottomRight),
+        ScrollType::TopEdge => Some(ScrollHint::TopEdge),
+        ScrollType::BottomEdge => Some(ScrollHint::BottomEdge),
+        ScrollType::LeftEdge => Some(ScrollHint::LeftEdge),
+        ScrollType::RightEdge => Some(ScrollHint::RightEdge),
+        ScrollType::Anywhere => None,
+    }
+}

platforms/macos/src/node.rs

@@ -30,6 +30,8 @@ use std::rc::{Rc, Weak};
 
 use crate::{context::Context, filters::filter, util::*};
 
+const SCROLL_TO_VISIBLE_ACTION: &str = "AXScrollToVisible";
+
 fn ns_role(node: &Node) -> &'static NSAccessibilityRole {
     let role = node.role();
     // TODO: Handle special cases.
@@ -961,6 +963,37 @@ declare_class!(
             .flatten()
         }
 
+        // We discovered through experimentation that when mixing the newer
+        // NSAccessibility protocols with the older informal protocol,
+        // the platform uses both protocols to discover which actions are
+        // available and then perform actions. That means our implementation
+        // of the legacy methods below only needs to cover actions not already
+        // handled by the newer methods.
+
+        #[method_id(accessibilityActionNames)]
+        fn action_names(&self) -> Id<NSArray<NSString>> {
+            let mut result = vec![];
+            self.resolve(|node| {
+                if node.supports_action(Action::ScrollIntoView, &filter) {
+                    result.push(ns_string!(SCROLL_TO_VISIBLE_ACTION).copy());
+                }
+            });
+            NSArray::from_vec(result)
+        }
+
+        #[method(accessibilityPerformAction:)]
+        fn perform_action(&self, action: &NSString) {
+            self.resolve_with_context(|node, context| {
+                if action == ns_string!(SCROLL_TO_VISIBLE_ACTION) {
+                    context.do_action(ActionRequest {
+                        action: Action::ScrollIntoView,
+                        target: node.id(),
+                        data: None,
+                    });
+                }
+            });
+        }
+
         #[method(isAccessibilitySelectorAllowed:)]
         fn is_selector_allowed(&self, selector: Sel) -> bool {
             self.resolve(|node| {
@@ -1043,6 +1076,8 @@ declare_class!(
                     || selector == sel!(isAccessibilityFocused)
                     || selector == sel!(accessibilityNotifiesWhenDestroyed)
                     || selector == sel!(isAccessibilitySelectorAllowed:)
+                    || selector == sel!(accessibilityActionNames)
+                    || selector == sel!(accessibilityPerformAction:)
             })
             .unwrap_or(false)
         }

platforms/unix/src/atspi/interfaces/component.rs

@@ -4,7 +4,7 @@
 // the LICENSE-MIT file), at your option.
 
 use accesskit_atspi_common::{PlatformNode, Rect};
-use atspi::{CoordType, Layer};
+use atspi::{CoordType, Layer, ScrollType};
 use zbus::{fdo, interface, names::OwnedUniqueName};
 
 use crate::atspi::{ObjectId, OwnedObjectAddress};
@@ -64,6 +64,10 @@ impl ComponentInterface {
         self.node.grab_focus().map_err(self.map_error())
     }
 
+    fn scroll_to(&self, scroll_type: ScrollType) -> fdo::Result<bool> {
+        self.node.scroll_to(scroll_type).map_err(self.map_error())
+    }
+
     fn scroll_to_point(&self, coord_type: CoordType, x: i32, y: i32) -> fdo::Result<bool> {
         self.node
             .scroll_to_point(coord_type, x, y)

platforms/windows/src/node.rs

@@ -381,6 +381,10 @@ impl NodeWrapper<'_> {
         self.0.is_required()
     }
 
+    fn is_scroll_item_pattern_supported(&self) -> bool {
+        self.0.supports_action(Action::ScrollIntoView, &filter)
+    }
+
     pub(crate) fn is_selection_item_pattern_supported(&self) -> bool {
         match self.0.role() {
             // TODO: tables (#29)
@@ -499,6 +503,7 @@ impl NodeWrapper<'_> {
     IInvokeProvider,
     IValueProvider,
     IRangeValueProvider,
+    IScrollItemProvider,
     ISelectionItemProvider,
     ISelectionProvider,
     ITextProvider
@@ -603,52 +608,59 @@ impl PlatformNode {
         self.resolve_with_context_for_text_pattern(|node, _| f(node))
     }
 
-    fn do_action<F>(&self, f: F) -> Result<()>
+    fn do_complex_action<F>(&self, f: F) -> Result<()>
     where
-        F: FnOnce() -> (Action, Option<ActionData>),
+        for<'a> F: FnOnce(Node<'a>) -> Result<Option<ActionRequest>>,
     {
         let context = self.upgrade_context()?;
         if context.is_placeholder.load(Ordering::SeqCst) {
-            return Ok(());
+            return Err(element_not_enabled());
         }
         let tree = context.read_tree();
-        let node_id = if let Some(id) = self.node_id {
-            if !tree.state().has_node(id) {
-                return Err(element_not_available());
-            }
-            id
-        } else {
-            tree.state().root_id()
-        };
-        drop(tree);
-        let (action, data) = f();
-        let request = ActionRequest {
-            target: node_id,
-            action,
-            data,
-        };
-        context.do_action(request);
+        let state = tree.state();
+        let node = self.node(state)?;
+        if let Some(request) = f(node)? {
+            drop(tree);
+            context.do_action(request);
+        }
         Ok(())
     }
 
+    fn do_action<F>(&self, f: F) -> Result<()>
+    where
+        F: FnOnce() -> (Action, Option<ActionData>),
+    {
+        self.do_complex_action(|node| {
+            if node.is_disabled() {
+                return Err(element_not_enabled());
+            }
+            let (action, data) = f();
+            Ok(Some(ActionRequest {
+                target: node.id(),
+                action,
+                data,
+            }))
+        })
+    }
+
     fn click(&self) -> Result<()> {
         self.do_action(|| (Action::Click, None))
     }
 
     fn set_selected(&self, selected: bool) -> Result<()> {
-        self.resolve_with_context(|node, context| {
+        self.do_complex_action(|node| {
             if node.is_disabled() {
                 return Err(element_not_enabled());
             }
             let wrapper = NodeWrapper(&node);
-            if selected != wrapper.is_selected() {
-                context.do_action(ActionRequest {
-                    action: Action::Click,
-                    target: node.id(),
-                    data: None,
-                });
+            if selected == wrapper.is_selected() {
+                return Ok(None);
             }
-            Ok(())
+            Ok(Some(ActionRequest {
+                action: Action::Click,
+                target: node.id(),
+                data: None,
+            }))
         })
     }
 
@@ -1004,6 +1016,17 @@ patterns! {
             })
         }
     )),
+    (UIA_ScrollItemPatternId, IScrollItemProvider, IScrollItemProvider_Impl, is_scroll_item_pattern_supported, (), (
+        fn ScrollIntoView(&self) -> Result<()> {
+            self.do_complex_action(|node| {
+                Ok(Some(ActionRequest {
+                    target: node.id(),
+                    action: Action::ScrollIntoView,
+                    data: None,
+                }))
+            })
+        }
+    )),
     (UIA_SelectionItemPatternId, ISelectionItemProvider, ISelectionItemProvider_Impl, is_selection_item_pattern_supported, (), (
         fn IsSelected(&self) -> Result<BOOL> {
             self.resolve(|node| {

platforms/windows/src/text.rs

@@ -5,7 +5,7 @@
 
 #![allow(non_upper_case_globals)]
 
-use accesskit::{Action, ActionData, ActionRequest};
+use accesskit::{Action, ActionData, ActionRequest, ScrollHint};
 use accesskit_consumer::{
     Node, TextPosition as Position, TextRange as Range, TreeState, WeakTextRange as WeakRange,
 };
@@ -582,9 +582,13 @@ impl ITextRangeProvider_Impl for PlatformRange_Impl {
                 range.end()
             };
             ActionRequest {
-                action: Action::ScrollIntoView,
                 target: position.inner_node().id(),
-                data: None,
+                action: Action::ScrollIntoView,
+                data: Some(ActionData::ScrollHint(if align_to_top.into() {
+                    ScrollHint::TopEdge
+                } else {
+                    ScrollHint::BottomEdge
+                })),
             }
         })
     }