Commit f37ba14a authored by Nektarios Paisios's avatar Nektarios Paisios Committed by Commit Bot

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

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: default avatarAaron Leventhal <aleventhal@chromium.org>
Reviewed-by: default avatarKevin Babbitt <kbabbitt@microsoft.com>
Auto-Submit: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748023}
parent 7c4c1500
......@@ -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"
......@@ -194,6 +197,62 @@ IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
host_view_parent));
}
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();
......@@ -288,10 +347,10 @@ IN_PROC_BROWSER_TEST_F(AccessibilityAuraLinuxBrowserTest,
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,
......
......@@ -3140,6 +3140,74 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
}
}
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;
......
......@@ -5,6 +5,7 @@
#ifndef UI_ACCESSIBILITY_AX_POSITION_H_
#define UI_ACCESSIBILITY_AX_POSITION_H_
#include <math.h>
#include <stdint.h>
#include <functional>
......@@ -2258,7 +2259,8 @@ class AXPosition {
// 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, ax::mojom::MoveDirection::kBackward);
} else if (boundary_behavior == AXBoundaryBehavior::StopAtAnchorBoundary) {
return CreatePositionAtStartOfAnchor();
}
......@@ -2323,7 +2325,8 @@ class AXPosition {
// 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, ax::mojom::MoveDirection::kForward);
} else if (boundary_behavior == AXBoundaryBehavior::StopAtAnchorBoundary) {
return CreatePositionAtEndOfAnchor();
}
......@@ -2768,13 +2771,16 @@ class AXPosition {
// 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()) {
......@@ -2812,23 +2818,76 @@ class AXPosition {
// 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
......
......@@ -263,6 +263,7 @@ const char* GetUniqueAccessibilityGTypeName(
}
void SetWeakGPtrToAtkObject(AtkObject** weak_pointer, AtkObject* new_value) {
DCHECK(weak_pointer);
if (*weak_pointer == new_value)
return;
......
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