Commit 1db2ac29 authored by Ian Prest's avatar Ian Prest Committed by Commit Bot

A11y: Fix a hang on placeholder text

The `CreateNextWord*Position` functions were not updating their iterator
in certain edge cases, which caused an infinite loop.  (Notably, the
`CreatePreviousWord*Position` functions *are* updating their iterators
in the same scenarios.)

The new test included in this change would hang before the fix.

The following is a detailed description of the error scenario:
-- Start with a AXTextPosition (at index 0) at a high level in the tree.
-- Call one of the `CreateNextWord*Position` functions.
-- These functions start by getting a "leaf" text position, which drills
   down deeper in the tree.  Specifically, they drill down into a node
   that includes placeholder text (which appears as part of their
   hypertext, but not in the hypertext of the outer node).
-- The function then tries to advance to the next word, and finds a
   valid word stop.
-- The end of this loop compares the new position against the original.
-- operator==/CompareTo must adjust the positions to be relative to the
   same node, which it does using repeated calls to GetParentPosition().
-- GetParentPosition() determines that the text-offset in the deep node
   is not valid in the outer node, so it resets the index to zero.
   (This workaround was added in CL:1661548.)
-- As a result, CompareTo determines that the two nodes are equal.
-- Therefore, we loop in CreateNextWord*Position.
-- Because the iterator was not updated, we loop infinitely.

Bug: 994871
Change-Id: I6e5880bfa7073e2ca672d281fca223e90e5fe288
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758590Reviewed-by: default avatarKevin Babbitt <kbabbitt@microsoft.com>
Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Ian Prest <iapres@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#688611}
parent 76e40a33
......@@ -824,4 +824,67 @@ TEST_F(BrowserAccessibilityTest, GetAuthorUniqueId) {
ASSERT_EQ(base::WideToUTF16(L"my_html_id"),
root_accessible->GetAuthorUniqueId());
}
TEST_F(BrowserAccessibilityTest, NextWordPositionWithHypertext) {
// Build a tree simulating an INPUT control with placeholder text.
ui::AXNodeData root;
root.id = 1;
root.role = ax::mojom::Role::kRootWebArea;
root.child_ids = {2};
ui::AXNodeData input;
input.id = 2;
input.role = ax::mojom::Role::kTextField;
input.child_ids = {3};
input.SetName("Search the web");
ui::AXNodeData static_text;
static_text.id = 3;
static_text.role = ax::mojom::Role::kStaticText;
static_text.child_ids = {4};
static_text.SetName("Search the web");
ui::AXNodeData inline_text;
inline_text.id = 4;
inline_text.role = ax::mojom::Role::kInlineTextBox;
inline_text.SetName("Search the web");
inline_text.AddIntListAttribute(ax::mojom::IntListAttribute::kWordStarts,
{0, 7, 11});
inline_text.AddIntListAttribute(ax::mojom::IntListAttribute::kWordEnds,
{6, 10, 14});
std::unique_ptr<BrowserAccessibilityManager> browser_accessibility_manager(
BrowserAccessibilityManager::Create(
MakeAXTreeUpdate(root, input, static_text, inline_text),
test_browser_accessibility_delegate_.get(),
new BrowserAccessibilityFactory()));
ASSERT_NE(nullptr, browser_accessibility_manager.get());
BrowserAccessibility* root_accessible =
browser_accessibility_manager->GetRoot();
ASSERT_NE(nullptr, root_accessible);
ASSERT_NE(0u, root_accessible->InternalChildCount());
BrowserAccessibility* input_accessible = root_accessible->InternalGetChild(0);
ASSERT_NE(nullptr, input_accessible);
// Create a text position at offset 0 in the input control
auto position =
input_accessible
->CreatePositionAt(0, ax::mojom::TextAffinity::kDownstream)
->AsTextPosition();
// On hypertext-based platforms (e.g., Windows IA2), the INPUT control will
// have a max-text-offset of zero, and so advancing to the next-word positions
// should return null positions. On non-hypertext platforms, the next-word
// positions will be non-null.
bool is_hypertext_platform = position->MaxTextOffset() == 0;
auto next_word_start = position->CreateNextWordStartPosition(
ui::AXBoundaryBehavior::CrossBoundary);
ASSERT_EQ(is_hypertext_platform, next_word_start->IsNullPosition());
auto next_word_end = position->CreateNextWordEndPosition(
ui::AXBoundaryBehavior::CrossBoundary);
ASSERT_EQ(is_hypertext_platform, next_word_end->IsNullPosition());
}
} // namespace content
......@@ -1346,6 +1346,7 @@ class AXPosition {
} else {
text_position->text_offset_ = static_cast<int>(*iterator);
text_position->affinity_ = ax::mojom::TextAffinity::kDownstream;
iterator++;
}
// Continue searching for the next word start until the next logical text
......@@ -1475,6 +1476,7 @@ class AXPosition {
} else {
text_position->text_offset_ = static_cast<int>(*iterator);
text_position->affinity_ = ax::mojom::TextAffinity::kDownstream;
iterator++;
}
// Continue searching for the next word end until the next logical text
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment