Commit 7bcbf8b1 authored by Nektarios Paisios's avatar Nektarios Paisios Committed by Commit Bot

Removes workaround for autofill popups from AXPosition

After https://crrev.com/c/2122718 has landed
which switches to using inner text instead of the kValue
attribute for retrieving the contents of text fields
in AXPosition, there is no longer a need to have a workaround for
computing the parent position of a text position that is
located inside an autofill popup that is a child
of a text field, since the text of the
popup, i.e. the autofill suggestion, would be part of inner text/.
Previously, the kValue attribute didn't reflect the value of any autofill suggestion.

RelNotes: Users of VoiceOver, NVDA and Jaws screen readers are able to use the
"Read Current Line" and "Read Current Word" commands to read the placeholder text in a text field.
<label>Name: <input type="text" placeholder="John Doe"></label>

R=dmazzoni@chromium.org, aleventhal@chromium.org

Change-Id: If57e012e8d16fa2b99e091aa5f86a0f145e266d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2125450Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Auto-Submit: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754524}
parent 2167e8d4
...@@ -743,14 +743,20 @@ TEST_F(BrowserAccessibilityTest, NextWordPositionWithHypertext) { ...@@ -743,14 +743,20 @@ TEST_F(BrowserAccessibilityTest, NextWordPositionWithHypertext) {
auto position = input_accessible->CreatePositionAt( auto position = input_accessible->CreatePositionAt(
0, ax::mojom::TextAffinity::kDownstream); 0, ax::mojom::TextAffinity::kDownstream);
// On platforms that expose IA2 or ATK hypertext, moving by word should have // On platforms that expose IA2 or ATK hypertext, moving by word should work
// no effect, i.e. return the same position, since the visible text is just a // the same as if the value of the text field is equal to the placeholder
// placeholder and should not appear in the input field's hypertext. // text.
//
// This is because visually the placeholder text appears in the text field in
// the same location as its value, and the user should be able to read it
// using standard screen reader commands, such as "read current word" and
// "read current line". Only once the user starts typing should the
// placeholder disappear.
auto next_word_start = position->CreateNextWordStartPosition( auto next_word_start = position->CreateNextWordStartPosition(
ui::AXBoundaryBehavior::CrossBoundary); ui::AXBoundaryBehavior::CrossBoundary);
ASSERT_TRUE(next_word_start->IsTextPosition());
if (position->MaxTextOffset() == 0) { if (position->MaxTextOffset() == 0) {
EXPECT_EQ(*position, *next_word_start); EXPECT_TRUE(next_word_start->IsNullPosition());
} else { } else {
EXPECT_EQ( EXPECT_EQ(
"TextPosition anchor_id=2 text_offset=7 affinity=downstream " "TextPosition anchor_id=2 text_offset=7 affinity=downstream "
...@@ -760,9 +766,8 @@ TEST_F(BrowserAccessibilityTest, NextWordPositionWithHypertext) { ...@@ -760,9 +766,8 @@ TEST_F(BrowserAccessibilityTest, NextWordPositionWithHypertext) {
auto next_word_end = position->CreateNextWordEndPosition( auto next_word_end = position->CreateNextWordEndPosition(
ui::AXBoundaryBehavior::CrossBoundary); ui::AXBoundaryBehavior::CrossBoundary);
ASSERT_TRUE(next_word_end->IsTextPosition());
if (position->MaxTextOffset() == 0) { if (position->MaxTextOffset() == 0) {
EXPECT_EQ(*position, *next_word_end); EXPECT_TRUE(next_word_end->IsNullPosition());
} else { } else {
EXPECT_EQ( EXPECT_EQ(
"TextPosition anchor_id=2 text_offset=6 affinity=downstream " "TextPosition anchor_id=2 text_offset=6 affinity=downstream "
......
...@@ -1093,11 +1093,13 @@ class AXPosition { ...@@ -1093,11 +1093,13 @@ class AXPosition {
return copy; return copy;
} }
// Blink doesn't always remove all deleted whitespace at the end of a // We stop at the last child that we can reach with the current text offset
// textarea even though it will have adjusted its value attribute, because // and ignore any remaining children. This is for defensive programming
// the extra layout objects are invisible. Therefore, we will stop at the // purposes, in case "MaxTextOffset" doesn't match the total length of all
// last child that we can reach with the current text offset and ignore any // our children. This may happen if, for example, there is a bug in the
// remaining children. // internal accessibility tree we get from the renderer. In contrast, the
// current offset could not be greater than the length of all our children
// because the position would have been invalid.
int current_offset = 0; int current_offset = 0;
int child_index = 0; int child_index = 0;
for (; child_index < copy->AnchorChildCount(); ++child_index) { for (; child_index < copy->AnchorChildCount(); ++child_index) {
...@@ -1827,18 +1829,6 @@ class AXPosition { ...@@ -1827,18 +1829,6 @@ class AXPosition {
AXPositionInstance parent_position = CreateTextPosition( AXPositionInstance parent_position = CreateTextPosition(
tree_id, parent_id, parent_offset, parent_affinity); tree_id, parent_id, parent_offset, parent_affinity);
if (parent_position->IsNullPosition()) {
// Workaround: When the autofill feature populates a text field, it
// doesn't immediately update its value, which causes the text inside
// the user-agent shadow DOM to be different than the text in the text
// field itself. As a result, the parent_offset calculated above might
// appear to be temporarily invalid.
// TODO(nektar): Fix this better by ensuring that the text field's
// hypertext is always kept up to date.
parent_position =
CreateTextPosition(tree_id, parent_id, 0 /* text_offset */,
ax::mojom::TextAffinity::kDownstream);
}
// If the current position is pointing at the end of its anchor, we need // If the current position is pointing at the end of its anchor, we need
// to check if the parent position has introduced ambiguity as to // to check if the parent position has introduced ambiguity as to
......
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