fix: Don't clamp character indices when converting a text selection to a range

consumer/src/text.rs

@@ -20,25 +20,6 @@ pub(crate) struct InnerPosition<'a> {
 
 impl<'a> InnerPosition<'a> {
     fn upgrade(tree_state: &'a TreeState, weak: WeakPosition, node_id: NodeId) -> Option<Self> {
-        let node = tree_state.node_by_id(node_id.with_same_tree(weak.node))?;
-        if node.role() != Role::TextRun {
-            return None;
-        }
-        let character_index = weak.character_index;
-        if character_index > node.data().character_lengths().len() {
-            return None;
-        }
-        Some(Self {
-            node,
-            character_index,
-        })
-    }
-
-    fn clamped_upgrade(
-        tree_state: &'a TreeState,
-        weak: WeakPosition,
-        node_id: NodeId,
-    ) -> Option<Self> {
         let node = tree_state.node_by_id(node_id.with_same_tree(weak.node))?;
         if node.role() != Role::TextRun {
             return None;
@@ -1450,19 +1431,16 @@ impl<'a> Node<'a> {
     pub fn text_selection(&self) -> Option<Range<'_>> {
         let id = self.id;
         self.data().text_selection().map(|selection| {
-            let anchor =
-                InnerPosition::clamped_upgrade(self.tree_state, selection.anchor, id).unwrap();
-            let focus =
-                InnerPosition::clamped_upgrade(self.tree_state, selection.focus, id).unwrap();
+            let anchor = InnerPosition::upgrade(self.tree_state, selection.anchor, id).unwrap();
+            let focus = InnerPosition::upgrade(self.tree_state, selection.focus, id).unwrap();
             Range::new(*self, anchor, focus)
         })
     }
 
     pub fn text_selection_anchor(&self) -> Option<Position<'_>> {
         let id = self.id;
         self.data().text_selection().map(|selection| {
-            let anchor =
-                InnerPosition::clamped_upgrade(self.tree_state, selection.anchor, id).unwrap();
+            let anchor = InnerPosition::upgrade(self.tree_state, selection.anchor, id).unwrap();
             Position {
                 root_node: *self,
                 inner: anchor,
@@ -1473,8 +1451,7 @@ impl<'a> Node<'a> {
     pub fn text_selection_focus(&self) -> Option<Position<'_>> {
         let id = self.id;
         self.data().text_selection().map(|selection| {
-            let focus =
-                InnerPosition::clamped_upgrade(self.tree_state, selection.focus, id).unwrap();
+            let focus = InnerPosition::upgrade(self.tree_state, selection.focus, id).unwrap();
             Position {
                 root_node: *self,
                 inner: focus,
@@ -1930,21 +1907,6 @@ mod tests {
         }
     }
 
-    fn multiline_past_end_selection() -> TextSelection {
-        use accesskit::TextPosition;
-
-        TextSelection {
-            anchor: TextPosition {
-                node: NodeId(9),
-                character_index: 3,
-            },
-            focus: TextPosition {
-                node: NodeId(9),
-                character_index: 3,
-            },
-        }
-    }
-
     fn multiline_wrapped_line_end_selection() -> TextSelection {
         use accesskit::TextPosition;
 
@@ -2802,14 +2764,6 @@ mod tests {
         assert!(node.text_position_from_global_utf16_index(100).is_none());
     }
 
-    #[test]
-    fn multiline_selection_clamping() {
-        let tree = main_multiline_tree(Some(multiline_past_end_selection()));
-        let state = tree.state();
-        let node = state.node_by_id(nid(NodeId(1))).unwrap();
-        let _ = node.text_selection().unwrap();
-    }
-
     #[test]
     fn range_property_value_map() {
         use super::RangePropertyValue;

platforms/windows/src/tests/mod.rs

@@ -3,7 +3,7 @@
 // the LICENSE-APACHE file) or the MIT license (found in
 // the LICENSE-MIT file), at your option.
 
-use accesskit::{ActionHandler, ActivationHandler};
+use accesskit::{ActionHandler, ActivationHandler, TreeUpdate};
 use once_cell::sync::Lazy;
 use std::{
     cell::RefCell,
@@ -31,6 +31,8 @@ use super::{
 
 const DEFAULT_TIMEOUT: Duration = Duration::from_secs(5);
 
+const WM_TEST_UPDATE_TREE: u32 = WM_APP;
+
 static WINDOW_CLASS_ATOM: Lazy<u16> = Lazy::new(|| {
     let class_name = w!("AccessKitTest");
 
@@ -129,6 +131,15 @@ extern "system" fn wndproc(window: HWND, message: u32, wparam: WPARAM, lparam: L
             update_window_focus_state(window, false);
             LRESULT(0)
         }
+        WM_TEST_UPDATE_TREE => {
+            let update = unsafe { Box::from_raw(lparam.0 as *mut TreeUpdate) };
+            let state = unsafe { &*get_window_state(window) };
+            let events = state.adapter.borrow_mut().update_if_active(|| *update);
+            if let Some(events) = events {
+                events.raise();
+            }
+            LRESULT(0)
+        }
         _ => unsafe { DefWindowProcW(window, message, wparam, lparam) },
     }
 }
@@ -177,6 +188,18 @@ impl Scope {
         let _ = unsafe { ShowWindow(self.window.0, SW_SHOW) };
         let _ = unsafe { SetForegroundWindow(self.window.0) };
     }
+
+    pub(crate) fn update_tree(&self, update: TreeUpdate) {
+        let boxed = Box::new(update);
+        unsafe {
+            SendMessageW(
+                self.window.0,
+                WM_TEST_UPDATE_TREE,
+                Some(WPARAM(0)),
+                Some(LPARAM(Box::into_raw(boxed) as _)),
+            )
+        };
+    }
 }
 
 // It's not safe to run these UI-related tests concurrently.

platforms/windows/src/tests/simple.rs

@@ -4,10 +4,13 @@
 // the LICENSE-MIT file), at your option.
 
 use accesskit::{
-    Action, ActionHandler, ActionRequest, ActivationHandler, Node, NodeId, Role, Tree, TreeId,
-    TreeUpdate,
+    Action, ActionHandler, ActionRequest, ActivationHandler, Node, NodeId, Role, TextDirection,
+    TextPosition, TextSelection, Tree, TreeId, TreeUpdate,
+};
+use windows::{
+    core::*,
+    Win32::{System::Variant::VARIANT, UI::Accessibility::*},
 };
-use windows::{core::*, Win32::UI::Accessibility::*};
 
 use super::*;
 
@@ -196,3 +199,136 @@ fn focus() -> Result<()> {
         Ok(())
     })
 }
+
+const TEXT_INPUT_ID: NodeId = NodeId(10);
+const TEXT_RUN_0_ID: NodeId = NodeId(20);
+const TEXT_RUN_1_ID: NodeId = NodeId(21);
+
+fn make_text_run(value: &str, character_lengths: &[u8], word_starts: &[u8]) -> Node {
+    let mut node = Node::new(Role::TextRun);
+    node.set_value(value);
+    node.set_character_lengths(character_lengths.to_vec().into_boxed_slice());
+    node.set_character_widths(vec![7.0; character_lengths.len()].into_boxed_slice());
+    node.set_character_positions(
+        (0..character_lengths.len())
+            .map(|i| i as f32 * 7.0)
+            .collect::<Vec<_>>()
+            .into_boxed_slice(),
+    );
+    node.set_word_starts(word_starts.to_vec().into_boxed_slice());
+    node.set_text_direction(TextDirection::LeftToRight);
+    node
+}
+
+fn two_line_text_tree() -> TreeUpdate {
+    let mut root = Node::new(Role::Window);
+    root.set_children(vec![TEXT_INPUT_ID]);
+
+    let mut text_input = Node::new(Role::TextInput);
+    text_input.add_action(Action::Focus);
+    text_input.set_children(vec![TEXT_RUN_0_ID, TEXT_RUN_1_ID]);
+    text_input.set_text_selection(TextSelection {
+        anchor: TextPosition {
+            node: TEXT_RUN_1_ID,
+            character_index: 6,
+        },
+        focus: TextPosition {
+            node: TEXT_RUN_1_ID,
+            character_index: 6,
+        },
+    });
+
+    let run_0 = make_text_run("Hello ", &[1; 6], &[0]);
+    let run_1 = make_text_run("world!", &[1; 6], &[0]);
+
+    TreeUpdate {
+        nodes: vec![
+            (WINDOW_ID, root),
+            (TEXT_INPUT_ID, text_input),
+            (TEXT_RUN_0_ID, run_0),
+            (TEXT_RUN_1_ID, run_1),
+        ],
+        tree: Some(Tree::new(WINDOW_ID)),
+        tree_id: TreeId::ROOT,
+        focus: TEXT_INPUT_ID,
+    }
+}
+
+fn one_line_text_update() -> TreeUpdate {
+    let mut text_input = Node::new(Role::TextInput);
+    text_input.add_action(Action::Focus);
+    text_input.set_children(vec![TEXT_RUN_0_ID]);
+    text_input.set_text_selection(TextSelection {
+        anchor: TextPosition {
+            node: TEXT_RUN_0_ID,
+            character_index: 11,
+        },
+        focus: TextPosition {
+            node: TEXT_RUN_0_ID,
+            character_index: 11,
+        },
+    });
+
+    let run_0 = make_text_run("Hello world", &[1; 11], &[0, 6]);
+
+    TreeUpdate {
+        nodes: vec![(TEXT_INPUT_ID, text_input), (TEXT_RUN_0_ID, run_0)],
+        tree: None,
+        tree_id: TreeId::ROOT,
+        focus: TEXT_INPUT_ID,
+    }
+}
+
+struct TextActivationHandler;
+
+impl ActivationHandler for TextActivationHandler {
+    fn request_initial_tree(&mut self) -> Option<TreeUpdate> {
+        Some(two_line_text_tree())
+    }
+}
+
+#[test]
+fn compare_endpoints_after_text_run_removed() -> Result<()> {
+    super::scope(
+        "Text reflow test",
+        TextActivationHandler {},
+        NullActionHandler {},
+        |s| {
+            s.show_and_focus_window();
+
+            let root = unsafe { s.uia.ElementFromHandle(s.window.0) }?;
+            let condition = unsafe {
+                s.uia.CreatePropertyCondition(
+                    UIA_ControlTypePropertyId,
+                    &VARIANT::from(UIA_EditControlTypeId.0),
+                )
+            }?;
+            let text_element = unsafe { root.FindFirst(TreeScope_Descendants, &condition) }?;
+
+            let pattern: IUIAutomationTextPattern =
+                unsafe { text_element.GetCurrentPatternAs(UIA_TextPatternId) }?;
+            let selection = unsafe { pattern.GetSelection() }?;
+            let old_range: IUIAutomationTextRange = unsafe { selection.GetElement(0) }?;
+
+            s.update_tree(one_line_text_update());
+
+            let new_selection = unsafe { pattern.GetSelection() }?;
+            let new_range: IUIAutomationTextRange = unsafe { new_selection.GetElement(0) }?;
+
+            let result = unsafe {
+                new_range.CompareEndpoints(
+                    TextPatternRangeEndpoint_Start,
+                    &old_range,
+                    TextPatternRangeEndpoint_Start,
+                )
+            };
+            assert!(
+                result.is_ok(),
+                "CompareEndpoints failed after text run removal: {:?}",
+                result.err()
+            );
+
+            Ok(())
+        },
+    )
+}