Skip to content

GitLab

T
tangled
Commit 42b4d13c authored by Adam Ettenberger's avatar Adam Ettenberger Committed by Commit Bot
Browse files
Options

Fix AXPosition Paragraph navigation can pick Ignored node endpoints 

This works is a continuation of improvements that Jacques Newman
had recently merged with this CL :
https://crrev.com/c/1810244

The problem with Paragraph navigation was that both the methods
|AtStartOfParagraph| and |AtEndOfParagraph| allowed an AXPosition
anchored to an Ignored node to be the start or end of a paragraph.

Adding a browsertest to exercise paragraph navigation when there
are Ignored nodes interleaved in the document which fails without
this patch.

The changes to |Create{Next|Previous}LeafTreePosition| does not change
behavior, there's no reason to copy |const base::RepeatingCallback| or
its bound parameters in this case.

Bug: 928948
Change-Id: I2cfee5481ffbea9a9a821cae6bcfc8e4652e847c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885021
Commit-Queue: Adam Ettenberger <adettenb@microsoft.com>
Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Reviewed-by: default avatarKurt Catti-Schmidt <kschmi@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#714739}
parent 14cf10e9
Hide whitespace changes
Inline Side-by-side
Showing with 227 additions and 11 deletions
content/browser/accessibility/ax_platform_node_textrangeprovider_win_browsertest.cc
View file @ 42b4d13c
......@@ -1477,6 +1477,53 @@ IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
/*expected_count*/ -1);
}
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
MoveByUnitParagraphWithAriaHiddenNodes) {
const std::string html_markup = R"HTML(<!DOCTYPE html>
<html>
<body>
<div>start</div>
<div>
1. Paragraph with hidden <span aria-hidden="true">
[IGNORED]
</span> inline in between
</div>
<div>
<span>2. Paragraph parts wrapped by</span> <span aria-hidden="true">
[IGNORED]
</span> <span>span with hidden inline in between</span>
</div>
<div>
<span>3. Paragraph before hidden block</span><div aria-hidden="true">
[IGNORED]
</div><span>4. Paragraph after hidden block</span>
</div>
<div>
<span aria-hidden="true">[IGNORED]</span><span>5. Paragraph with leading
and trailing hidden span</span><span aria-hidden="true">[IGNORED]</span>
</div>
<div>
<div aria-hidden="true">[IGNORED]</div><span>6. Paragraph with leading
and trailing hidden block</span><div aria-hidden="true">[IGNORED]</div>
</div>
<div>end</div>
</body>
</html>)HTML";
const std::vector<const wchar_t*> paragraphs = {
L"start",
L"1. Paragraph with hidden inline in between",
L"2. Paragraph parts wrapped by span with hidden inline in between",
L"3. Paragraph before hidden block",
L"4. Paragraph after hidden block",
L"5. Paragraph with leading and trailing hidden span",
L"6. Paragraph with leading and trailing hidden block",
L"end",
};
AssertMoveByUnitForMarkup(TextUnit_Paragraph, html_markup, paragraphs);
}
IN_PROC_BROWSER_TEST_F(AXPlatformNodeTextRangeProviderWinBrowserTest,
IFrameTraversal) {
LoadInitialAccessibilityTreeFromUrl(embedded_test_server()->GetURL(
......
ui/accessibility/ax_node_position_unittest.cc
View file @ 42b4d13c
......@@ -1580,6 +1580,166 @@ TEST_F(AXPositionTest,
EXPECT_FALSE(text_position10->AtStartOfParagraph());
}
TEST_F(AXPositionTest, AtStartOrEndOfParagraphWithIgnoredNodes) {
// This test updates the tree structure to test a specific edge case -
// At{Start|End}OfParagraph when there are ignored nodes present near
// a paragraph boundary.
// ++1 kRootWebArea isLineBreakingObject
// ++++2 kGenericContainer ignored
// ++++++3 kStaticText ignored
// ++++++++4 kInlineTextBox "ignored text" ignored
// ++++5 kGenericContainer
// ++++++6 kStaticText
// ++++++++7 kInlineTextBox "some"
// ++++++++8 kInlineTextBox " "
// ++++++++9 kInlineTextBox "text"
// ++++10 kGenericContainer ignored
// ++++++11 kStaticText ignored
// ++++++++12 kInlineTextBox "ignored text" ignored
AXNodePosition::SetTree(nullptr);
AXNodeData root_data;
root_data.id = 1;
root_data.role = ax::mojom::Role::kRootWebArea;
root_data.AddBoolAttribute(ax::mojom::BoolAttribute::kIsLineBreakingObject,
true);
AXNodeData container_data_a;
container_data_a.id = 2;
container_data_a.role = ax::mojom::Role::kGenericContainer;
container_data_a.AddState(ax::mojom::State::kIgnored);
AXNodeData static_text_data_a;
static_text_data_a.id = 3;
static_text_data_a.role = ax::mojom::Role::kStaticText;
static_text_data_a.SetName("ignored text");
static_text_data_a.AddState(ax::mojom::State::kIgnored);
AXNodeData inline_text_data_a;
inline_text_data_a.id = 4;
inline_text_data_a.role = ax::mojom::Role::kInlineTextBox;
inline_text_data_a.SetName("ignored text");
inline_text_data_a.AddState(ax::mojom::State::kIgnored);
AXNodeData container_data_b;
container_data_b.id = 5;
container_data_b.role = ax::mojom::Role::kGenericContainer;
AXNodeData static_text_data_b;
static_text_data_b.id = 6;
static_text_data_b.role = ax::mojom::Role::kStaticText;
static_text_data_b.SetName("some text");
AXNodeData inline_text_data_b_1;
inline_text_data_b_1.id = 7;
inline_text_data_b_1.role = ax::mojom::Role::kInlineTextBox;
inline_text_data_b_1.SetName("some");
AXNodeData inline_text_data_b_2;
inline_text_data_b_2.id = 8;
inline_text_data_b_2.role = ax::mojom::Role::kInlineTextBox;
inline_text_data_b_2.SetName(" ");
AXNodeData inline_text_data_b_3;
inline_text_data_b_3.id = 9;
inline_text_data_b_3.role = ax::mojom::Role::kInlineTextBox;
inline_text_data_b_3.SetName("text");
AXNodeData container_data_c;
container_data_c.id = 10;
container_data_c.role = ax::mojom::Role::kGenericContainer;
container_data_c.AddState(ax::mojom::State::kIgnored);
AXNodeData static_text_data_c;
static_text_data_c.id = 11;
static_text_data_c.role = ax::mojom::Role::kStaticText;
static_text_data_c.SetName("ignored text");
static_text_data_c.AddState(ax::mojom::State::kIgnored);
AXNodeData inline_text_data_c;
inline_text_data_c.id = 12;
inline_text_data_c.role = ax::mojom::Role::kInlineTextBox;
inline_text_data_c.SetName("ignored text");
inline_text_data_c.AddState(ax::mojom::State::kIgnored);
root_data.child_ids = {container_data_a.id, container_data_b.id,
container_data_c.id};
container_data_a.child_ids = {static_text_data_a.id};
static_text_data_a.child_ids = {inline_text_data_a.id};
container_data_b.child_ids = {static_text_data_b.id};
static_text_data_b.child_ids = {inline_text_data_b_1.id,
inline_text_data_b_2.id,
inline_text_data_b_3.id};
container_data_c.child_ids = {static_text_data_c.id};
static_text_data_c.child_ids = {inline_text_data_c.id};
std::unique_ptr<AXTree> new_tree = CreateAXTree(
{root_data, container_data_a, container_data_b, container_data_c,
static_text_data_a, static_text_data_b, static_text_data_c,
inline_text_data_a, inline_text_data_b_1, inline_text_data_b_2,
inline_text_data_b_3, inline_text_data_c});
TestPositionType text_position1 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_a.id, 0 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position1->AtEndOfParagraph());
EXPECT_FALSE(text_position1->AtStartOfParagraph());
TestPositionType text_position2 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_a.id, 12 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position2->AtEndOfParagraph());
EXPECT_FALSE(text_position2->AtStartOfParagraph());
TestPositionType text_position3 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_b_1.id, 0 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position3->AtEndOfParagraph());
EXPECT_TRUE(text_position3->AtStartOfParagraph());
TestPositionType text_position4 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_b_1.id, 4 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position4->AtEndOfParagraph());
EXPECT_FALSE(text_position4->AtStartOfParagraph());
TestPositionType text_position5 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_b_2.id, 0 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position5->AtEndOfParagraph());
EXPECT_FALSE(text_position5->AtStartOfParagraph());
TestPositionType text_position6 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_b_2.id, 1 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position6->AtEndOfParagraph());
EXPECT_FALSE(text_position6->AtStartOfParagraph());
TestPositionType text_position7 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_b_3.id, 0 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position7->AtEndOfParagraph());
EXPECT_FALSE(text_position7->AtStartOfParagraph());
TestPositionType text_position8 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_b_3.id, 4 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_TRUE(text_position8->AtEndOfParagraph());
EXPECT_FALSE(text_position8->AtStartOfParagraph());
TestPositionType text_position9 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_c.id, 0 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position9->AtEndOfParagraph());
EXPECT_FALSE(text_position9->AtStartOfParagraph());
TestPositionType text_position10 = AXNodePosition::CreateTextPosition(
new_tree->data().tree_id, inline_text_data_c.id, 12 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
EXPECT_FALSE(text_position10->AtEndOfParagraph());
EXPECT_FALSE(text_position10->AtStartOfParagraph());
}
TEST_F(AXPositionTest, LowestCommonAncestor) {
TestPositionType null_position = AXNodePosition::CreateNullPosition();
ASSERT_NE(nullptr, null_position);
......
ui/accessibility/ax_position.h
View file @ 42b4d13c
......@@ -477,7 +477,8 @@ class AXPosition {
// position from the one representing the end of the previous paragraph.
// A position |AsLeafTextPosition| is the start of a paragraph if all of the
// following are true :
// 1. The current leaf text position must be at the start of an anchor.
// 1. The current leaf text position must be an unignored position at
// the start of an anchor.
// 2. The current position is not whitespace only, unless it is also
// the first leaf text position within the document.
// 3. Either (a) the current leaf text position is the first leaf text
......@@ -493,8 +494,9 @@ class AXPosition {
NOTREACHED();
return false;
case AXPositionKind::TEXT_POSITION: {
// 1. The current leaf text position must be at the start of an anchor.
if (!text_position->AtStartOfAnchor())
// 1. The current leaf text position must be an unignored position at
// the start of an anchor.
if (text_position->IsIgnored() || !text_position->AtStartOfAnchor())
return false;
// 2. The current position is not whitespace only, unless it is also
......@@ -529,7 +531,8 @@ class AXPosition {
// There's a chance that |CreatePreviousTextAnchorPosition| will
// return whitespace that should be appended to a previous paragraph
// rather than separating two pieces of the current paragraph.
} while (previous_text_position->IsInWhiteSpace());
} while (previous_text_position->IsInWhiteSpace() ||
previous_text_position->IsIgnored());
return previous_text_position->IsNullPosition();
}
}
......@@ -542,7 +545,8 @@ class AXPosition {
// position from the one representing the start of the next paragraph.
// A position |AsLeafTextPosition| is the end of a paragraph if all of the
// following are true :
// 1. The current leaf text position must be at the end of an anchor.
// 1. The current leaf text position must be an unignored position at
// the end of an anchor.
// 2. Either (a) the current leaf text position is the last leaf text
// position in the document, or (b) there are no line breaking
// objects between it and the next leaf text position except when
......@@ -562,8 +566,9 @@ class AXPosition {
NOTREACHED();
return false;
case AXPositionKind::TEXT_POSITION: {
// 1. The current leaf text position must be at the end of an anchor.
if (!text_position->AtEndOfAnchor())
// 1. The current leaf text position must be an unignored position at
// the end of an anchor.
if (text_position->IsIgnored() || !text_position->AtEndOfAnchor())
return false;
// 2. Either (a) the current leaf text position is the last leaf text
......@@ -586,8 +591,12 @@ class AXPosition {
const AbortMovePredicate abort_move_predicate =
base::BindRepeating(&AbortMoveAtParagraphBoundary,
std::ref(crossed_potential_boundary_token));
AXPositionInstance next_text_position =
text_position->CreateNextTextAnchorPosition(abort_move_predicate);
AXPositionInstance next_text_position = text_position->Clone();
do {
next_text_position = next_text_position->CreateNextTextAnchorPosition(
abort_move_predicate);
} while (next_text_position->IsIgnored());
if (next_text_position->IsNullPosition())
return true;
......@@ -2613,7 +2622,7 @@ class AXPosition {
// Creates a tree position using the next text-only node as its anchor.
// Assumes that text-only nodes are leaf nodes.
AXPositionInstance CreateNextLeafTreePosition(
const AbortMovePredicate abort_predicate) const {
const AbortMovePredicate& abort_predicate) const {
AXPositionInstance next_leaf =
AsTreePosition()->CreateNextAnchorPosition(abort_predicate);
while (!next_leaf->IsNullPosition() && next_leaf->AnchorChildCount()) {
......@@ -2627,7 +2636,7 @@ class AXPosition {
// Creates a tree position using the previous text-only node as its anchor.
// Assumes that text-only nodes are leaf nodes.
AXPositionInstance CreatePreviousLeafTreePosition(
const AbortMovePredicate abort_predicate) const {
const AbortMovePredicate& abort_predicate) const {
AXPositionInstance previous_leaf =
AsTreePosition()->CreatePreviousAnchorPosition(abort_predicate);
while (!previous_leaf->IsNullPosition() &&
......
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