Merge to M81: Computes the correct ancestor position when a leaf position is within an embedded object
On platforms that support embedded object characters, such as IA2 and ATK, we were not
computing the correct ancestor position when trying to compare two leaf text positions.
If the leaf text position is within an embedded object, we need to adjust the ancestor position
to point to after the embedded object character, if the leaf position is not at the start of the embedded object.
This is per the IA2 Spec.
Test steps are in the attached bug.
R=dmazzoni@chromium.org, kbabbitt@microsoft.com
TBR=nektar@chromium.org
(cherry picked from commit f37ba14ae0f0f7835402613497642f0c00d6e4e2)
Bug: 1057831
Change-Id: Iabc44586ae35c4bb927655707366bf3b81b79fc9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081527
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Kevin Babbitt <kbabbitt@microsoft.com>
Auto-Submit: Nektarios Paisios <nektar@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#748023}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2119762
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/branch-heads/4044@{#852}
Cr-Branched-From: a6d9daf149a473ceea37f629c41d4527bf2055bd-refs/heads/master@{#737173}
diff --git a/content/browser/accessibility/accessibility_auralinux_browsertest.cc b/content/browser/accessibility/accessibility_auralinux_browsertest.cc
index 4426268..776a52d 100644
--- a/content/browser/accessibility/accessibility_auralinux_browsertest.cc
+++ b/content/browser/accessibility/accessibility_auralinux_browsertest.cc
@@ -5,6 +5,9 @@
#include <atk/atk.h>
#include <dlfcn.h>
+#include <string>
+#include <vector>
+
#include "base/bind_helpers.h"
#include "base/macros.h"
#include "build/build_config.h"
@@ -195,6 +198,62 @@
}
IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
+ TestTextAtOffsetWithBoundaryCharacterAndEmbeddedObject) {
+ LoadInitialAccessibilityTreeFromHtml(R"HTML(<!DOCTYPE html>
+ <div contenteditable>
+ Before<img alt="image">after.
+ </div>
+ )HTML");
+
+ AtkObject* document = GetRendererAccessible();
+ ASSERT_EQ(1, atk_object_get_n_accessible_children(document));
+
+ AtkObject* contenteditable = atk_object_ref_accessible_child(document, 0);
+ ASSERT_NE(nullptr, contenteditable);
+ ASSERT_EQ(ATK_ROLE_SECTION, atk_object_get_role(contenteditable));
+ ASSERT_TRUE(ATK_IS_TEXT(contenteditable));
+
+ AtkText* contenteditable_text = ATK_TEXT(contenteditable);
+ int character_count = atk_text_get_character_count(contenteditable_text);
+ ASSERT_EQ(13, character_count);
+
+ const base::string16 embedded_character(
+ 1, ui::AXPlatformNodeAuraLinux::kEmbeddedCharacter);
+ const std::vector<const std::string> expected_hypertext = {
+ "B", "e", "f", "o", "r", "e", base::UTF16ToUTF8(embedded_character),
+ "a", "f", "t", "e", "r", "."};
+
+ // "Before".
+ //
+ // The embedded object character representing the image is at offset 6.
+ for (int i = 0; i <= 6; ++i) {
+ CheckTextAtOffset(contenteditable_text, i, ATK_TEXT_BOUNDARY_CHAR, i,
+ (i + 1), expected_hypertext[i].c_str());
+ }
+
+ // "after.".
+ //
+ // Note that according to the ATK Spec, an offset that is equal to
+ // "character_count" is not permitted.
+ for (int i = 7; i < character_count; ++i) {
+ CheckTextAtOffset(contenteditable_text, i, ATK_TEXT_BOUNDARY_CHAR, i,
+ (i + 1), expected_hypertext[i].c_str());
+ }
+
+ ASSERT_EQ(3, atk_object_get_n_accessible_children(contenteditable));
+ // The image is the second child.
+ AtkObject* image = atk_object_ref_accessible_child(contenteditable, 1);
+ ASSERT_NE(nullptr, image);
+ ASSERT_EQ(ATK_ROLE_IMAGE, atk_object_get_role(image));
+
+ // The alt text of the image is not navigable as text.
+ ASSERT_FALSE(ATK_IS_TEXT(image));
+
+ g_object_unref(image);
+ g_object_unref(contenteditable_text);
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
TestMultilingualTextAtOffsetWithBoundaryCharacter) {
AtkText* atk_text = SetUpInputField();
ASSERT_NE(nullptr, atk_text);
@@ -288,10 +347,10 @@
int n_characters = atk_text_get_character_count(atk_text);
ASSERT_LT(newline_offset, n_characters);
- const base::string16 string16_embed(
+ const base::string16 embedded_character(
1, ui::AXPlatformNodeAuraLinux::kEmbeddedCharacter);
std::string expected_string = "Game theory is \"the study of " +
- base::UTF16ToUTF8(string16_embed) +
+ base::UTF16ToUTF8(embedded_character) +
" of conflict and\n";
for (int i = 0; i <= newline_offset; ++i) {
CheckTextAtOffset(atk_text, i, ATK_TEXT_BOUNDARY_LINE_START, 0,
diff --git a/content/browser/accessibility/accessibility_win_browsertest.cc b/content/browser/accessibility/accessibility_win_browsertest.cc
index 6383e53..a3c5ed2 100644
--- a/content/browser/accessibility/accessibility_win_browsertest.cc
+++ b/content/browser/accessibility/accessibility_win_browsertest.cc
@@ -3139,6 +3139,74 @@
}
IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
+ TestTextAtOffsetWithBoundaryCharacterAndEmbeddedObject) {
+ LoadInitialAccessibilityTreeFromHtml(R"HTML(<!DOCTYPE html>
+ <div contenteditable>
+ Before<img alt="image">after.
+ </div>
+ )HTML");
+
+ Microsoft::WRL::ComPtr<IAccessible> document(GetRendererAccessible());
+ std::vector<base::win::ScopedVariant> document_children =
+ GetAllAccessibleChildren(document.Get());
+ ASSERT_EQ(1u, document_children.size());
+
+ Microsoft::WRL::ComPtr<IAccessible2> contenteditable;
+ ASSERT_HRESULT_SUCCEEDED(QueryIAccessible2(
+ GetAccessibleFromVariant(document.Get(), document_children[0].AsInput())
+ .Get(),
+ &contenteditable));
+
+ Microsoft::WRL::ComPtr<IAccessibleText> contenteditable_text;
+ ASSERT_HRESULT_SUCCEEDED(contenteditable.As(&contenteditable_text));
+
+ LONG n_characters;
+ ASSERT_HRESULT_SUCCEEDED(
+ contenteditable_text->get_nCharacters(&n_characters));
+ ASSERT_EQ(13, n_characters);
+
+ const base::string16 embedded_character(
+ 1, BrowserAccessibilityComWin::kEmbeddedCharacter);
+ const std::wstring expected_hypertext =
+ L"Before" + base::UTF16ToWide(embedded_character) + L"after.";
+
+ // "Before".
+ //
+ // The embedded object character representing the image is at offset 6.
+ for (LONG i = 0; i <= 6; ++i) {
+ CheckTextAtOffset(contenteditable_text, i, IA2_TEXT_BOUNDARY_CHAR, i,
+ (i + 1), std::wstring(1, expected_hypertext[i]));
+ }
+
+ // "after.".
+ //
+ // Note that according to the IA2 Spec, an offset that is equal to
+ // "n_characters" is not permitted.
+ for (LONG i = 7; i < n_characters; ++i) {
+ CheckTextAtOffset(contenteditable_text, i, IA2_TEXT_BOUNDARY_CHAR, i,
+ (i + 1), std::wstring(1, expected_hypertext[i]));
+ }
+
+ std::vector<base::win::ScopedVariant> contenteditable_children =
+ GetAllAccessibleChildren(contenteditable.Get());
+ ASSERT_EQ(3u, contenteditable_children.size());
+ // The image is the second child.
+ Microsoft::WRL::ComPtr<IAccessible2> image;
+ ASSERT_HRESULT_SUCCEEDED(QueryIAccessible2(
+ GetAccessibleFromVariant(contenteditable.Get(),
+ contenteditable_children[1].AsInput())
+ .Get(),
+ &image));
+ LONG image_role = 0;
+ ASSERT_HRESULT_SUCCEEDED(image->role(&image_role));
+ ASSERT_EQ(ROLE_SYSTEM_GRAPHIC, image_role);
+
+ // The alt text of the image is not navigable as text.
+ Microsoft::WRL::ComPtr<IAccessibleText> image_text;
+ EXPECT_HRESULT_FAILED(image.As(&image_text));
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
TestTextAtOffsetWithBoundaryWord) {
Microsoft::WRL::ComPtr<IAccessibleText> input_text;
SetUpInputField(&input_text);
diff --git a/ui/accessibility/ax_position.h b/ui/accessibility/ax_position.h
index 2568812..6f3b33c 100644
--- a/ui/accessibility/ax_position.h
+++ b/ui/accessibility/ax_position.h
@@ -5,6 +5,7 @@
#ifndef UI_ACCESSIBILITY_AX_POSITION_H_
#define UI_ACCESSIBILITY_AX_POSITION_H_
+#include <math.h>
#include <stdint.h>
#include <memory>
@@ -2103,7 +2104,8 @@
// be in the shadow DOM if the original position was not.
const AXNodeType* common_anchor = tree_position->LowestCommonAnchor(*this);
if (GetAnchor() == common_anchor) {
- tree_position = tree_position->CreateAncestorPosition(common_anchor);
+ tree_position = tree_position->CreateAncestorPosition(
+ common_anchor, AXTextBoundaryDirection::kBackwards);
} else if (boundary_behavior == AXBoundaryBehavior::StopAtAnchorBoundary) {
return CreatePositionAtStartOfAnchor();
}
@@ -2168,7 +2170,8 @@
// be in the shadow DOM if the original position was not.
const AXNodeType* common_anchor = tree_position->LowestCommonAnchor(*this);
if (GetAnchor() == common_anchor) {
- tree_position = tree_position->CreateAncestorPosition(common_anchor);
+ tree_position = tree_position->CreateAncestorPosition(
+ common_anchor, AXTextBoundaryDirection::kForwards);
} else if (boundary_behavior == AXBoundaryBehavior::StopAtAnchorBoundary) {
return CreatePositionAtEndOfAnchor();
}
@@ -2587,13 +2590,16 @@
// Normalize any text positions at the end of an anchor to equivalent
// positions at the start of the next anchor.
AXPositionInstance normalized_this_position = Clone();
- if (normalized_this_position->IsTextPosition())
+ if (normalized_this_position->IsTextPosition()) {
normalized_this_position =
normalized_this_position->AsLeafTextPositionBeforeCharacter();
+ }
+
AXPositionInstance normalized_other_position = other.Clone();
- if (normalized_other_position->IsTextPosition())
+ if (normalized_other_position->IsTextPosition()) {
normalized_other_position =
normalized_other_position->AsLeafTextPositionBeforeCharacter();
+ }
if (normalized_this_position->IsNullPosition()) {
if (normalized_other_position->IsNullPosition()) {
@@ -2631,23 +2637,76 @@
// If each position has an uncommon ancestor node, we can compare those
// instead of needing to compute ancestor positions.
if (!our_ancestors.empty() && !other_ancestors.empty()) {
+ AXPositionInstance this_uncommon_tree_position = CreateTreePosition(
+ GetTreeID(our_ancestors.top()), GetAnchorID(our_ancestors.top()),
+ 0 /*child_index*/);
int this_uncommon_ancestor_index =
- CreateTreePosition(GetTreeID(our_ancestors.top()),
- GetAnchorID(our_ancestors.top()),
- 0 /*child_index*/)
- ->AnchorIndexInParent();
+ this_uncommon_tree_position->AnchorIndexInParent();
+ AXPositionInstance other_uncommon_tree_position = CreateTreePosition(
+ GetTreeID(other_ancestors.top()), GetAnchorID(other_ancestors.top()),
+ 0 /*child_index*/);
int other_uncommon_ancestor_index =
- CreateTreePosition(GetTreeID(other_ancestors.top()),
- GetAnchorID(other_ancestors.top()),
- 0 /*child_index*/)
- ->AnchorIndexInParent();
- DCHECK(this_uncommon_ancestor_index != other_uncommon_ancestor_index);
+ other_uncommon_tree_position->AnchorIndexInParent();
+ DCHECK_NE(this_uncommon_ancestor_index, other_uncommon_ancestor_index)
+ << "Deepest uncommon ancestors should truly be uncommon, i.e. not "
+ "the same.";
int result = this_uncommon_ancestor_index - other_uncommon_ancestor_index;
+ // On platforms that support embedded objects, if a text position is
+ // within an embedded object and if it is not at the start of that object,
+ // the resulting ancestor position should be adjusted to point after the
+ // embedded object. Otherwise, assistive software will not be able to get
+ // out of the embedded object if its text is not editable when navigating
+ // by character.
+ //
+ // For example, look at the following accessibility tree and the two
+ // example text positions together with their equivalent ancestor
+ // positions.
+ // ++1 kRootWebArea
+ // ++++2 kTextField "Before<embedded_object>after"
+ // ++++++3 kStaticText "Before"
+ // ++++++++4 kInlineTextBox "Before"
+ // ++++++5 kImage "Test image"
+ // ++++++6 kStaticText "after"
+ // ++++++++7 kInlineTextBox "after"
+ //
+ // Note that the alt text of an image cannot be navigated with cursor
+ // left/right, even when the rest of the contents are in a
+ // contenteditable.
+ //
+ // Ancestor position should not be adjusted:
+ // TextPosition anchor_id=kImage text_offset=0 affinity=downstream
+ // annotated_text=<T>est image AncestorTextPosition anchor_id=kTextField
+ // text_offset=6 affinity=downstream
+ // annotated_text=Before<embedded_object>after
+ //
+ // Ancestor position should be adjusted:
+ // TextPosition anchor_id=kImage text_offset=1 affinity=downstream
+ // annotated_text=T<e>st image AncestorTextPosition anchor_id=kTextField
+ // text_offset=7 affinity=downstream
+ // annotated_text=Beforeembedded_object<a>fter
+ //
+ // Note that since the adjustment to the distance between the ancestor
+ // positions could at most be by one, we skip doing this check if the
+ // ancestor positions have a distance of more than one since it can never
+ // change the outcome of the comparison. Note too that if both ancestor
+ // positions need to be adjusted, the adjustments will cancel out.
+ if (abs(result) == 1) {
+ if (!normalized_this_position->AtStartOfAnchor() &&
+ this_uncommon_tree_position->IsEmbeddedObjectInParent()) {
+ result += 1;
+ }
+ if (!normalized_other_position->AtStartOfAnchor() &&
+ other_uncommon_tree_position->IsEmbeddedObjectInParent()) {
+ result -= 1;
+ }
+ }
+
#if DCHECK_IS_ON()
// Validate the optimization.
int slow_result = SlowCompareTo(other).value();
- DCHECK((result < 0 && slow_result < 0) ||
+ DCHECK((result == 0 && slow_result == 0) ||
+ (result < 0 && slow_result < 0) ||
(result > 0 && slow_result > 0));
#endif
diff --git a/ui/accessibility/platform/ax_platform_node_auralinux.cc b/ui/accessibility/platform/ax_platform_node_auralinux.cc
index 814853e..7e7abdc 100644
--- a/ui/accessibility/platform/ax_platform_node_auralinux.cc
+++ b/ui/accessibility/platform/ax_platform_node_auralinux.cc
@@ -258,6 +258,7 @@
}
void SetWeakGPtrToAtkObject(AtkObject** weak_pointer, AtkObject* new_value) {
+ DCHECK(weak_pointer);
if (*weak_pointer == new_value)
return;
