Commit 67487b0d authored by Benjamin Beaudry's avatar Benjamin Beaudry Committed by Commit Bot

Fix DCHECK in AXPosition::CompareTo with rich text fields

An accessible node that represents generated content (e.g., the ::before
selector) is completely ignored in AXNodePosition::MaxTextOffset and
AXNodePosition::GetText. Both methods return the
ax::mojom::StringAttribute::kValue's length and value, respectively.
Since this attribute's value doesn't include the value of the generated
content (because this generated content is either before or after the
main content of the text field), we return a wrong maximum text offset
and text.

To fix this, we should only use ax::mojom::StringAttribute::kValue's
length and value when the node has no child. When a node has a child, we
need to compute the maximum text offset by summing the maximum text
offset values of all of its children. The same applies for
AXNodePosition::GetText.

Bug: 928948
Change-Id: I624779a3e372d19ac2036de09fe9dbf1c4892867
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037965
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@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@{#739228}
parent 05a86be2
...@@ -173,10 +173,14 @@ base::string16 AXNodePosition::GetText() const { ...@@ -173,10 +173,14 @@ base::string16 AXNodePosition::GetText() const {
const AXNode* anchor = GetAnchor(); const AXNode* anchor = GetAnchor();
DCHECK(anchor); DCHECK(anchor);
text = GetAnchor()->data().GetString16Attribute( // TODO(nektar): Replace with PlatformChildCount when AXNodePosition and
ax::mojom::StringAttribute::kValue); // BrowserAccessibilityPosition will make one.
if (!text.empty()) if (!AnchorChildCount()) {
return text; text =
anchor->data().GetString16Attribute(ax::mojom::StringAttribute::kValue);
if (!text.empty())
return text;
}
if (anchor->IsText()) { if (anchor->IsText()) {
return anchor->data().GetString16Attribute( return anchor->data().GetString16Attribute(
...@@ -225,10 +229,14 @@ int AXNodePosition::MaxTextOffset() const { ...@@ -225,10 +229,14 @@ int AXNodePosition::MaxTextOffset() const {
const AXNode* anchor = GetAnchor(); const AXNode* anchor = GetAnchor();
DCHECK(anchor); DCHECK(anchor);
base::string16 value = GetAnchor()->data().GetString16Attribute( // TODO(nektar): Replace with PlatformChildCount when AXNodePosition and
ax::mojom::StringAttribute::kValue); // BrowserAccessibilityPosition will make one.
if (!value.empty()) if (!AnchorChildCount()) {
return value.length(); base::string16 value =
anchor->data().GetString16Attribute(ax::mojom::StringAttribute::kValue);
if (!value.empty())
return value.length();
}
if (anchor->IsText()) { if (anchor->IsText()) {
return anchor->data() return anchor->data()
......
...@@ -1068,6 +1068,64 @@ TEST_F(AXPositionTest, GetMaxTextOffsetUpdate) { ...@@ -1068,6 +1068,64 @@ TEST_F(AXPositionTest, GetMaxTextOffsetUpdate) {
AssertTextLengthEquals(new_tree.get(), root_data.id, 24); AssertTextLengthEquals(new_tree.get(), root_data.id, 24);
} }
TEST_F(AXPositionTest, GetMaxTextOffsetAndGetTextWithGeneratedContent) {
// ++1 kRootWebArea
// ++++2 kTextField
// ++++++3 kStaticText
// ++++++++4 kInlineTextBox
// ++++++5 kStaticText
// ++++++++6 kInlineTextBox
AXNodeData root_1;
AXNodeData text_field_2;
AXNodeData static_text_3;
AXNodeData inline_box_4;
AXNodeData static_text_5;
AXNodeData inline_box_6;
root_1.id = 1;
text_field_2.id = 2;
static_text_3.id = 3;
inline_box_4.id = 4;
static_text_5.id = 5;
inline_box_6.id = 6;
root_1.role = ax::mojom::Role::kRootWebArea;
root_1.child_ids = {text_field_2.id};
text_field_2.role = ax::mojom::Role::kTextField;
text_field_2.SetValue("3.14");
text_field_2.child_ids = {static_text_3.id, static_text_5.id};
static_text_3.role = ax::mojom::Role::kStaticText;
static_text_3.SetName("Placeholder from generated content");
static_text_3.child_ids = {inline_box_4.id};
inline_box_4.role = ax::mojom::Role::kInlineTextBox;
inline_box_4.SetName("Placeholder from generated content");
static_text_5.role = ax::mojom::Role::kStaticText;
static_text_5.SetName("3.14");
static_text_5.child_ids = {inline_box_6.id};
inline_box_6.role = ax::mojom::Role::kInlineTextBox;
inline_box_6.SetName("3.14");
std::unique_ptr<AXTree> new_tree =
CreateAXTree({root_1, text_field_2, static_text_3, inline_box_4,
static_text_5, inline_box_6});
AXNodePosition::SetTree(new_tree.get());
TestPositionType text_position = AXNodePosition::CreateTextPosition(
tree_.data().tree_id, text_field_2.id, 0 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
ASSERT_NE(nullptr, text_position);
EXPECT_TRUE(text_position->IsTextPosition());
EXPECT_EQ(38, text_position->MaxTextOffset());
EXPECT_EQ(base::WideToUTF16(L"Placeholder from generated content3.14"),
text_position->GetText());
}
TEST_F(AXPositionTest, AtStartOfAnchorWithNullPosition) { TEST_F(AXPositionTest, AtStartOfAnchorWithNullPosition) {
TestPositionType null_position = AXNodePosition::CreateNullPosition(); TestPositionType null_position = AXNodePosition::CreateNullPosition();
ASSERT_NE(nullptr, null_position); ASSERT_NE(nullptr, null_position);
......
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