Skip to content

Commit

Permalink
Implemented finding sentence boundaries on all platforms
Browse files Browse the repository at this point in the history 
This patch:
1. Implements functionality in AXPosition that finds sentence boundaries.
2. Ensures that consistent boundaries are provided on all platforms
by routing all "find boundary" requests through AXPosition.
3. Stops relying on the BrowserAccessibility delegate to find text boundaries.
This will enable Views to also make use of AXPosition.
4. Stops using ax_text_utils, with the aim of removing this file in
a followup patch.

R=aleventhal@chromium.org

AX-Relnotes: n/a.
Change-Id: I90d946c4e5cd901ab4d5626f0d226e0f49b9b87b
Bug: 1049261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3275773
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#948482}
  • Loading branch information
Nektarios Paisios authored and Chromium LUCI CQ committed Dec 6, 2021
1 parent 9d571e0 commit 10c66f6
Show file tree
Hide file tree
Showing 25 changed files with 2,303 additions and 1,055 deletions.
4 changes: 2 additions & 2 deletions content/browser/accessibility/accessibility_action_browsertest.cc
View file
Expand Up @@ -474,7 +474,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityActionBrowserTest, TextareaSetValue) {
target->CreateTextPositionAt(0);
BrowserAccessibility::AXPosition end_of_line_1 =
start_position->CreateNextLineEndPosition(
ui::AXBoundaryBehavior::CrossBoundary);
ui::AXBoundaryBehavior::kCrossBoundary);
EXPECT_EQ(5, end_of_line_1->text_offset());
#endif
}
Expand Down Expand Up @@ -508,7 +508,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityActionBrowserTest,
target->CreateTextPositionAt(0);
BrowserAccessibility::AXPosition end_of_line_1 =
start_position->CreateNextLineEndPosition(
ui::AXBoundaryBehavior::CrossBoundary);
ui::AXBoundaryBehavior::kCrossBoundary);
EXPECT_EQ(5, end_of_line_1->text_offset());
#endif
}
Expand Down
2 changes: 1 addition & 1 deletion content/browser/accessibility/accessibility_auralinux_browsertest.cc
View file
Expand Up @@ -460,7 +460,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
}

IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
TestParagraphTextAtOffsetWithBoundarySentence) {
DISABLED_TestParagraphTextAtOffsetWithBoundarySentence) {
LoadInitialAccessibilityTreeFromHtml(std::string(
R"HTML(<!DOCTYPE html>
<html>
Expand Down
117 changes: 81 additions & 36 deletions content/browser/accessibility/accessibility_win_browsertest.cc
View file
Expand Up @@ -1774,7 +1774,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
Microsoft::WRL::ComPtr<IAccessibleText> input_text;
SetUpScrollableInputField(&input_text);

int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
constexpr LONG visible_characters_start = 21;
LONG n_characters;
ASSERT_HRESULT_SUCCEEDED(input_text->get_nCharacters(&n_characters));
Expand Down Expand Up @@ -1969,7 +1969,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
Microsoft::WRL::ComPtr<IAccessibleText> input_text;
SetUpScrollableInputTypeSearchField(&input_text);

int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
constexpr LONG visible_characters_start = 21;
LONG n_characters;
ASSERT_HRESULT_SUCCEEDED(input_text->get_nCharacters(&n_characters));
Expand Down Expand Up @@ -2618,7 +2618,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest, TestSetSelection) {
AccessibilityNotificationWaiter waiter(
shell()->web_contents(), ui::kAXModeComplete,
ax::mojom::Event::kTextSelectionChanged);
int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
start_offset = 0;
end_offset = contents_string_length;
EXPECT_HRESULT_FAILED(input_text->setSelection(1, start_offset, end_offset));
Expand Down Expand Up @@ -2654,7 +2654,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest, TestSetSelectionRanges) {
LONG n_ranges = 1;
IA2Range* ranges =
reinterpret_cast<IA2Range*>(CoTaskMemAlloc(sizeof(IA2Range)));
int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
ranges[0].anchor = ax_input.Get();
ranges[0].anchorOffset = -1;
ranges[0].active = ax_input.Get();
Expand Down Expand Up @@ -2859,7 +2859,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest, TestMultiLineSetSelection) {
// There is no selection, just a caret.
EXPECT_EQ(E_INVALIDARG, hr);

int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
AccessibilityNotificationWaiter waiter(
shell()->web_contents(), ui::kAXModeComplete,
ax::mojom::Event::kTextSelectionChanged);
Expand Down Expand Up @@ -2896,7 +2896,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
Microsoft::WRL::ComPtr<IAccessible2_4> ax_textarea;
ASSERT_HRESULT_SUCCEEDED(textarea_text.As(&ax_textarea));

int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
LONG n_ranges = 1;
IA2Range* ranges =
reinterpret_cast<IA2Range*>(CoTaskMemAlloc(sizeof(IA2Range)));
Expand Down Expand Up @@ -3471,7 +3471,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
EXPECT_EQ(nullptr, text.Get());
hr = input_text->get_textAtOffset(invalid_offset, IA2_TEXT_BOUNDARY_SENTENCE,
&start_offset, &end_offset, text.Receive());
EXPECT_EQ(S_FALSE, hr);
EXPECT_EQ(E_INVALIDARG, hr);
EXPECT_EQ(0, start_offset);
EXPECT_EQ(0, end_offset);
EXPECT_EQ(nullptr, text.Get());
Expand Down Expand Up @@ -3508,7 +3508,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
hr = input_text->get_textAtOffset(IA2_TEXT_OFFSET_LENGTH,
IA2_TEXT_BOUNDARY_SENTENCE, &start_offset,
&end_offset, text.Receive());
EXPECT_EQ(S_FALSE, hr);
EXPECT_EQ(E_INVALIDARG, hr);
EXPECT_EQ(0, start_offset);
EXPECT_EQ(0, end_offset);
EXPECT_EQ(nullptr, text.Get());
Expand Down Expand Up @@ -3578,7 +3578,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
hr = textarea_text->get_textAtOffset(
invalid_offset, IA2_TEXT_BOUNDARY_SENTENCE, &start_offset, &end_offset,
text.Receive());
EXPECT_EQ(S_FALSE, hr);
EXPECT_EQ(E_INVALIDARG, hr);
EXPECT_EQ(0, start_offset);
EXPECT_EQ(0, end_offset);
EXPECT_EQ(nullptr, text.Get());
Expand Down Expand Up @@ -3617,7 +3617,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
hr = textarea_text->get_textAtOffset(
IA2_TEXT_OFFSET_LENGTH, IA2_TEXT_BOUNDARY_SENTENCE, &start_offset,
&end_offset, text.Receive());
EXPECT_EQ(S_FALSE, hr);
EXPECT_EQ(E_INVALIDARG, hr);
EXPECT_EQ(0, start_offset);
EXPECT_EQ(0, end_offset);
EXPECT_EQ(nullptr, text.Get());
Expand All @@ -3643,7 +3643,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
Microsoft::WRL::ComPtr<IAccessibleText> input_text;
SetUpInputField(&input_text);

int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
for (LONG offset = 0; offset < contents_string_length; ++offset) {
std::wstring expected_text(1, InputContentsString()[offset]);
LONG expected_start_offset = offset;
Expand Down Expand Up @@ -3671,7 +3671,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
Microsoft::WRL::ComPtr<IAccessibleText> textarea_text;
SetUpTextareaField(&textarea_text);

int contents_string_length = InputContentsString().size();
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
for (LONG offset = 0; offset < contents_string_length; ++offset) {
std::wstring expected_text(1, TextAreaContentsString()[offset]);
LONG expected_start_offset = offset;
Expand Down Expand Up @@ -3859,7 +3859,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
CheckTextAtOffset(input_text, 39, IA2_TEXT_BOUNDARY_WORD, 38, 40, L", ");

// Trailing final punctuation should not be part of the last word.
int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
CheckTextAtOffset(input_text, 40, IA2_TEXT_BOUNDARY_WORD, 40, 44, L"like");
CheckTextAtOffset(input_text, 41, IA2_TEXT_BOUNDARY_WORD, 40, 44, L"like");
CheckTextAtOffset(input_text, 44, IA2_TEXT_BOUNDARY_WORD, 44,
Expand Down Expand Up @@ -3924,7 +3924,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
CheckTextAtOffset(textarea_text, 39, IA2_TEXT_BOUNDARY_WORD, 38, 40, L", ");

// Trailing final punctuation should not be part of the last word.
int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
CheckTextAtOffset(textarea_text, 40, IA2_TEXT_BOUNDARY_WORD, 40, 44, L"like");
CheckTextAtOffset(textarea_text, 41, IA2_TEXT_BOUNDARY_WORD, 40, 44, L"like");
CheckTextAtOffset(textarea_text, 44, IA2_TEXT_BOUNDARY_WORD, 44,
Expand Down Expand Up @@ -3967,33 +3967,78 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
}

IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
TestTextAtOffsetWithBoundarySentence) {
DISABLED_TestTextAtOffsetWithBoundarySentence) {
Microsoft::WRL::ComPtr<IAccessibleText> input_text;
SetUpInputField(&input_text);

// Sentence navigation is not currently implemented.
LONG start_offset = 0;
LONG end_offset = 0;
base::win::ScopedBstr text;
HRESULT hr =
input_text->get_textAtOffset(5, IA2_TEXT_BOUNDARY_SENTENCE, &start_offset,
&end_offset, text.Receive());
EXPECT_EQ(S_FALSE, hr);
const LONG contents_string_length =
static_cast<LONG>(InputContentsString().size());
const std::wstring expected_text = base::SysUTF8ToWide(InputContentsString());
for (LONG offset = 0; offset < contents_string_length; ++offset) {
CheckTextAtOffset(input_text, offset, IA2_TEXT_BOUNDARY_SENTENCE, 0,
contents_string_length, expected_text);
}

// Test special offsets.
CheckTextAtOffset(input_text, IA2_TEXT_OFFSET_CARET,
IA2_TEXT_BOUNDARY_SENTENCE, 0, contents_string_length,
expected_text);
{
LONG start_offset = 0;
LONG end_offset = 0;
base::win::ScopedBstr text;
HRESULT hr = input_text->get_textAtOffset(
IA2_TEXT_OFFSET_LENGTH, IA2_TEXT_BOUNDARY_SENTENCE, &start_offset,
&end_offset, text.Receive());
EXPECT_EQ(E_INVALIDARG, hr);
EXPECT_EQ(0, start_offset);
EXPECT_EQ(0, end_offset);
EXPECT_EQ(nullptr, text.Get());
}
}

IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
TestMultiLineTextAtOffsetWithBoundarySentence) {
DISABLED_TestMultiLineTextAtOffsetWithBoundarySentence) {
Microsoft::WRL::ComPtr<IAccessibleText> textarea_text;
SetUpTextareaField(&textarea_text);

// Sentence navigation is not currently implemented.
LONG start_offset = 0;
LONG end_offset = 0;
base::win::ScopedBstr text;
HRESULT hr = textarea_text->get_textAtOffset(25, IA2_TEXT_BOUNDARY_SENTENCE,
&start_offset, &end_offset,
text.Receive());
EXPECT_EQ(S_FALSE, hr);
const LONG contents_string_length =
static_cast<LONG>(TextAreaContentsString().size());
const std::vector<LONG> sentence_starts{0, 23, 24, 31, 32};
const std::vector<LONG> sentence_ends{23, 24, 31, 32, contents_string_length};
size_t sentence_index = 0;
for (LONG offset = 0; offset < contents_string_length &&
sentence_index < sentence_starts.size();
++offset) {
if (offset == sentence_starts[sentence_index + 1])
++sentence_index;
LONG expected_start_offset = sentence_starts[sentence_index];
LONG expected_end_offset = sentence_ends[sentence_index];
const std::wstring expected_text =
base::SysUTF8ToWide(TextAreaContentsString().substr(
sentence_starts[sentence_index],
(sentence_ends[sentence_index] - sentence_starts[sentence_index])));
CheckTextAtOffset(textarea_text, offset, IA2_TEXT_BOUNDARY_SENTENCE,
expected_start_offset, expected_end_offset,
expected_text);
}

// Test special offsets.
CheckTextAtOffset(textarea_text, IA2_TEXT_OFFSET_CARET,
IA2_TEXT_BOUNDARY_SENTENCE, 32, contents_string_length,
base::SysUTF8ToWide(TextAreaContentsString().substr(32)));
{
LONG start_offset = 0;
LONG end_offset = 0;
base::win::ScopedBstr text;
HRESULT hr = textarea_text->get_textAtOffset(
IA2_TEXT_OFFSET_LENGTH, IA2_TEXT_BOUNDARY_SENTENCE, &start_offset,
&end_offset, text.Receive());
EXPECT_EQ(E_INVALIDARG, hr);
EXPECT_EQ(0, start_offset);
EXPECT_EQ(0, end_offset);
EXPECT_EQ(nullptr, text.Get());
}
}

IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
Expand All @@ -4002,7 +4047,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
SetUpInputField(&input_text);

// Single line text fields should return the whole text.
int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
CheckTextAtOffset(input_text, 0, IA2_TEXT_BOUNDARY_LINE, 0,
contents_string_length,
base::SysUTF8ToWide(InputContentsString()));
Expand All @@ -4029,7 +4074,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
L"WebKit \n");

// Last line does not have a trailing newline.
int contents_string_length = static_cast<int>(InputContentsString().size());
LONG contents_string_length = static_cast<LONG>(InputContentsString().size());
CheckTextAtOffset(textarea_text, 32, IA2_TEXT_BOUNDARY_LINE, 32,
contents_string_length, L"\"KHTML, like\".");

Expand Down Expand Up @@ -4067,8 +4112,8 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
// Blink represents the blank line with a newline character, so in total there
// should be two more newlines. The second newline is not part of the HTML
// value attribute however.
int contents_string_length =
static_cast<int>(InputContentsString().size()) + 1;
LONG contents_string_length =
static_cast<LONG>(InputContentsString().size()) + 1;
CheckTextAtOffset(textarea_text, 32, IA2_TEXT_BOUNDARY_LINE, 32,
contents_string_length, L"\"KHTML, like\".\n");
CheckTextAtOffset(textarea_text, 46, IA2_TEXT_BOUNDARY_LINE, 32,
Expand Down
4 changes: 2 additions & 2 deletions content/browser/accessibility/ax_platform_node_textrangeprovider_win_browsertest.cc
View file
Expand Up @@ -2758,7 +2758,7 @@ IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
/*expected_count*/ -1);
ASSERT_HRESULT_SUCCEEDED(
text_range_provider->ExpandToEnclosingUnit(TextUnit_Word));
EXPECT_UIA_TEXTRANGE_EQ(text_range_provider, L"\xA0\n");
EXPECT_UIA_TEXTRANGE_EQ(text_range_provider, L"\xA0");

// Case 2: test on range that includes the whitespace and the following word.
GetTextRangeProviderFromTextNode(*node, &text_range_provider);
Expand All @@ -2771,7 +2771,7 @@ IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
/*expected_count*/ 1);
ASSERT_HRESULT_SUCCEEDED(
text_range_provider->ExpandToEnclosingUnit(TextUnit_Word));
EXPECT_UIA_TEXTRANGE_EQ(text_range_provider, L"\xA0\n");
EXPECT_UIA_TEXTRANGE_EQ(text_range_provider, L"\xA0");

// Case 3: test on degenerate range after whitespace.
node = FindNode(ax::mojom::Role::kStaticText, "3.14");
Expand Down
43 changes: 0 additions & 43 deletions content/browser/accessibility/browser_accessibility.cc
View file
Expand Up @@ -106,26 +106,6 @@ BrowserAccessibility* GetTextFieldInnerEditorElement(
return nullptr;
}

int GetBoundaryTextOffsetInsideBaseAnchor(
ax::mojom::MoveDirection direction,
const BrowserAccessibility::AXPosition& base,
const BrowserAccessibility::AXPosition& position) {
if (base->GetAnchor() == position->GetAnchor())
return position->text_offset();

// If the position is outside the anchor of the base position, then return
// the first or last position in the same direction.
switch (direction) {
case ax::mojom::MoveDirection::kNone:
NOTREACHED();
return position->text_offset();
case ax::mojom::MoveDirection::kBackward:
return base->CreatePositionAtStartOfAnchor()->text_offset();
case ax::mojom::MoveDirection::kForward:
return base->CreatePositionAtEndOfAnchor()->text_offset();
}
}

} // namespace

bool BrowserAccessibility::IsValid() const {
Expand Down Expand Up @@ -1188,29 +1168,6 @@ std::string BrowserAccessibility::SubtreeToStringHelper(size_t level) {
return result;
}

absl::optional<int> BrowserAccessibility::FindTextBoundary(
ax::mojom::TextBoundary boundary,
int offset,
ax::mojom::MoveDirection direction,
ax::mojom::TextAffinity affinity) const {
const AXPosition position = CreateTextPositionAt(offset, affinity);

// On Windows and Linux ATK, searching for a text boundary should always stop
// at the boundary of the current object.
auto boundary_behavior = ui::AXBoundaryBehavior::StopAtAnchorBoundary;
// On Windows and Linux ATK, it is standard text navigation behavior to stop
// if we are searching in the backwards direction and the current position is
// already at the required text boundary.
DCHECK_NE(direction, ax::mojom::MoveDirection::kNone);
if (direction == ax::mojom::MoveDirection::kBackward)
boundary_behavior = ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary;

return GetBoundaryTextOffsetInsideBaseAnchor(
direction, position,
position->CreatePositionAtTextBoundary(boundary, direction,
boundary_behavior));
}

const std::vector<gfx::NativeViewAccessible>
BrowserAccessibility::GetUIADirectChildrenInRange(
ui::AXPlatformNodeDelegate* start,
Expand Down
6 changes: 0 additions & 6 deletions content/browser/accessibility/browser_accessibility.h
View file
Expand Up @@ -510,12 +510,6 @@ class CONTENT_EXPORT BrowserAccessibility : public ui::AXPlatformNodeDelegate {
int GetIndexInParent() override;
gfx::AcceleratedWidget GetTargetForNativeAccessibilityEvent() override;

absl::optional<int> FindTextBoundary(
ax::mojom::TextBoundary boundary,
int offset,
ax::mojom::MoveDirection direction,
ax::mojom::TextAffinity affinity) const override;

const std::vector<gfx::NativeViewAccessible> GetUIADirectChildrenInRange(
ui::AXPlatformNodeDelegate* start,
ui::AXPlatformNodeDelegate* end) override;
Expand Down

0 comments on commit 10c66f6

Please sign in to comment.