Skip to content

GitLab

T
tangled
Commit fc270131 authored by Martin Robinson's avatar Martin Robinson Committed by Commit Bot
Browse files
Options

Fix failing DCHECK in ATKText boundary analysis 

In order to fix this issue, we need to undo a change where list markers
were rolled into the text of the list item. This also exposes an issue
with boundary analysis. It seems that the platform-independent boundary
analysis code can sometimes return text offsets in anchors other than
the one corresponding to the ATK text. This change addresses those two
related issues in order to fix the ATKText interface.

We also remove one test for an issue that is no longer present with
these changes.

Bug: 962498
Change-Id: I7f4e7284e5d212d4eb1b56dad51944cbe03457f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630686Reviewed-by: default avatarNektarios Paisios <nektar@chromium.org>
Commit-Queue: Martin Robinson <mrobinson@igalia.com>
Cr-Commit-Position: refs/heads/master@{#664168}
parent 6119fb2c
Hide whitespace changes
Inline Side-by-side
Showing with 71 additions and 70 deletions
content/browser/accessibility/accessibility_auralinux_browsertest.cc
View file @ fc270131
......@@ -244,36 +244,6 @@ IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
g_object_unref(atk_text);
}
IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
TestAtkTextGetTextCrash) {
LoadInitialAccessibilityTreeFromHtml(
R"HTML(<!DOCTYPE html>
<html>
<body>
<ul><li>Text</li></ul>
</body>
</html>)HTML");
// Retrieve the AtkObject interface for the document node.
AtkObject* document = GetRendererAccessible();
EXPECT_EQ(1, atk_object_get_n_accessible_children(document));
AtkObject* list = atk_object_ref_accessible_child(document, 0);
EXPECT_TRUE(ATK_IS_TEXT(list));
// There should be no crash when atk_text_get_text_at_offset returns an
// empty value.
int start_offset = -1, end_offset = -1;
gchar* text = atk_text_get_text_at_offset(ATK_TEXT(list), 0,
ATK_TEXT_BOUNDARY_WORD_START,
&start_offset, &end_offset);
ASSERT_EQ(text, nullptr);
ASSERT_EQ(start_offset, -1);
ASSERT_EQ(end_offset, -1);
g_object_unref(list);
}
#if defined(ATK_CHECK_VERSION) && ATK_CHECK_VERSION(2, 30, 0)
#define ATK_230
#endif
......@@ -754,24 +724,40 @@ IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest, TestAtkTextListItem) {
R"HTML(<!DOCTYPE html>
<html>
<body>
<li>Text</li>
<li>Text 1</li>
<li>Text 2</li>
<li>Text 3</li>
</body>
</html>)HTML");
// Retrieve the AtkObject interface for the document node.
AtkObject* document = GetRendererAccessible();
EXPECT_EQ(1, atk_object_get_n_accessible_children(document));
AtkObject* list_item = atk_object_ref_accessible_child(document, 0);
EXPECT_EQ(3, atk_object_get_n_accessible_children(document));
AtkObject* list_item_1 = atk_object_ref_accessible_child(document, 0);
AtkObject* list_item_2 = atk_object_ref_accessible_child(document, 1);
EXPECT_TRUE(ATK_IS_TEXT(list_item_1));
EXPECT_TRUE(ATK_IS_TEXT(list_item));
const base::string16 string16_embed(
1, ui::AXPlatformNodeAuraLinux::kEmbeddedCharacter);
std::string expected_string = base::UTF16ToUTF8(string16_embed) + "Text 1";
// The text of the list item should include the list marker as an embedded
// object.
gchar* text = atk_text_get_text(ATK_TEXT(list_item_1), 0, -1);
ASSERT_STREQ(text, expected_string.c_str());
g_free(text);
text = atk_text_get_text_at_offset(
ATK_TEXT(list_item_2), 0, ATK_TEXT_BOUNDARY_WORD_START, nullptr, nullptr);
ASSERT_STREQ(text, base::UTF16ToUTF8(string16_embed).c_str());
g_free(text);
// The text of the list item should include the list marker and the text of
// the item itself.
gchar* text = atk_text_get_text(ATK_TEXT(list_item), 0, -1);
ASSERT_STREQ(text, "\xE2\x80\xA2 Text");
text = atk_text_get_text_at_offset(
ATK_TEXT(list_item_2), 1, ATK_TEXT_BOUNDARY_WORD_START, nullptr, nullptr);
ASSERT_STREQ(text, "Text ");
g_free(text);
g_object_unref(list_item);
g_object_unref(list_item_1);
}
IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
......
content/browser/accessibility/browser_accessibility.cc
View file @ fc270131
......@@ -39,6 +39,27 @@ BrowserAccessibility::BrowserAccessibility()
BrowserAccessibility::~BrowserAccessibility() {}
namespace {
int GetBoundaryTextOffsetInsideBaseAnchor(
ui::TextBoundaryDirection direction,
const BrowserAccessibilityPosition::AXPositionInstance& base,
const BrowserAccessibilityPosition::AXPositionInstance& 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 ui::BACKWARDS_DIRECTION:
return base->CreatePositionAtStartOfAnchor()->text_offset();
case ui::FORWARDS_DIRECTION:
return base->CreatePositionAtEndOfAnchor()->text_offset();
}
}
} // namespace
void BrowserAccessibility::Init(BrowserAccessibilityManager* manager,
ui::AXNode* node) {
manager_ = manager;
......@@ -1229,15 +1250,15 @@ base::Optional<int> BrowserAccessibility::FindTextBoundary(
CreatePositionAt(static_cast<int>(offset), affinity);
switch (direction) {
case ui::BACKWARDS_DIRECTION:
return position
->CreatePreviousWordStartPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary)
->text_offset();
return GetBoundaryTextOffsetInsideBaseAnchor(
direction, position,
position->CreatePreviousWordStartPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary));
case ui::FORWARDS_DIRECTION:
return position
->CreateNextWordStartPosition(
ui::AXBoundaryBehavior::StopAtAnchorBoundary)
->text_offset();
return GetBoundaryTextOffsetInsideBaseAnchor(
direction, position,
position->CreateNextWordStartPosition(
ui::AXBoundaryBehavior::StopAtAnchorBoundary));
}
}
case ui::AXTextBoundary::kWordStartOrEnd: {
......@@ -1245,15 +1266,15 @@ base::Optional<int> BrowserAccessibility::FindTextBoundary(
CreatePositionAt(static_cast<int>(offset), affinity);
switch (direction) {
case ui::BACKWARDS_DIRECTION:
return position
->CreatePreviousWordStartPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary)
->text_offset();
return GetBoundaryTextOffsetInsideBaseAnchor(
direction, position,
position->CreatePreviousWordStartPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary));
case ui::FORWARDS_DIRECTION:
return position
->CreateNextWordEndPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary)
->text_offset();
return GetBoundaryTextOffsetInsideBaseAnchor(
direction, position,
position->CreateNextWordEndPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary));
}
}
case ui::AXTextBoundary::kLineStart: {
......@@ -1261,15 +1282,15 @@ base::Optional<int> BrowserAccessibility::FindTextBoundary(
CreatePositionAt(static_cast<int>(offset), affinity);
switch (direction) {
case ui::BACKWARDS_DIRECTION:
return position
->CreatePreviousLineStartPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary)
->text_offset();
return GetBoundaryTextOffsetInsideBaseAnchor(
direction, position,
position->CreatePreviousLineStartPosition(
ui::AXBoundaryBehavior::StopIfAlreadyAtBoundary));
case ui::FORWARDS_DIRECTION:
return position
->CreateNextLineStartPosition(
ui::AXBoundaryBehavior::StopAtAnchorBoundary)
->text_offset();
return GetBoundaryTextOffsetInsideBaseAnchor(
direction, position,
position->CreateNextLineStartPosition(
ui::AXBoundaryBehavior::StopAtAnchorBoundary));
}
}
default:
......
ui/accessibility/platform/ax_platform_node_auralinux.cc
View file @ fc270131
......@@ -3734,11 +3734,6 @@ void AXPlatformNodeAuraLinux::GetFloatAttributeInGValue(
}
}
bool AXPlatformNodeAuraLinux::IsTextOnlyObject() const {
return GetData().role == ax::mojom::Role::kListMarker ||
AXPlatformNodeBase::IsTextOnlyObject();
}
void AXPlatformNodeAuraLinux::AddAttributeToList(const char* name,
const char* value,
AtkAttributeSet** attributes) {
......
ui/accessibility/platform/ax_platform_node_auralinux.h
View file @ fc270131
......@@ -143,7 +143,6 @@ class AX_EXPORT AXPlatformNodeAuraLinux : public AXPlatformNodeBase {
base::Optional<base::OffsetAdjuster::Adjustments> text_unicode_adjustments_ =
base::nullopt;
bool IsTextOnlyObject() const override;
void AddAttributeToList(const char* name,
const char* value,
PlatformAttributeList* attributes) override;
......
ui/accessibility/platform/ax_platform_node_base.h
View file @ fc270131
......@@ -209,7 +209,7 @@ class AX_EXPORT AXPlatformNodeBase : public AXPlatformNode {
// Returns true if this node has role of StaticText, LineBreak, or
// InlineTextBox
virtual bool IsTextOnlyObject() const;
bool IsTextOnlyObject() const;
// Returns true if the node is an editable text field.
bool IsPlainTextField() const;
......
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