0

Enable AXBoundsForRange attribute on plain text fields.

Aside from patching the guard returning nil for anything except static text
this change also fixes a few issues that get in a way:
- incorrect coordinate space handling (-[BrowserAccessibilityCocoa rectInScreen:]
expects its input to be in root frame coordinate space)
- redundant (and seemingly wrong) code in `GetInnerTextRangeBoundsRectInSubtree()` that
causes incorrect results for text fields.

Bug: 1131376
AX-Relnotes: VoiceOver and Zoom now correctly highlight the text in a text field.
Change-Id: Ie14be2b0d36c420735397496da78cb1f53e7c26a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424090
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Abigail Klein <abigailbklein@google.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815232}
This commit is contained in:
Ievgen Rysai
2020-10-08 17:58:29 +00:00
committed by Commit Bot
parent 90253a3d46
commit c689eca4b6
6 changed files with 160 additions and 13 deletions

@ -1176,6 +1176,7 @@ Estimote, Inc. <*@estimote.com>
Facebook, Inc. <*@fb.com>
Facebook, Inc. <*@oculus.com>
Google Inc. <*@google.com>
Grammarly, Inc. <*@grammarly.com>
Hewlett-Packard Development Company, L.P. <*@hp.com>
HyperConnect Inc. <*@hpcnt.com>
IBM Inc. <*@*.ibm.com>

@ -704,12 +704,6 @@ gfx::Rect BrowserAccessibility::GetInnerTextRangeBoundsRectInSubtree(
coordinate_system, clipping_behavior, offscreen_result);
}
if (IsPlainTextField() && InternalChildCount() == 1) {
return GetTextContainerForPlainTextField(*this)->RelativeToAbsoluteBounds(
GetInlineTextRect(start_offset, end_offset, GetInnerText().length()),
coordinate_system, clipping_behavior, offscreen_result);
}
gfx::Rect bounds;
int child_offset_in_parent = 0;
for (InternalChildIterator it = InternalChildrenBegin();

@ -111,6 +111,7 @@ id AXTextMarkerRangeFrom(id anchor_textmarker, id focus_textmarker);
- (NSString*)valueForRange:(NSRange)range;
- (NSAttributedString*)attributedValueForRange:(NSRange)range;
- (NSRect)frameForRange:(NSRange)range;
// Internally-used property.
@property(nonatomic, readonly) NSPoint origin;

@ -2626,6 +2626,14 @@ id content::AXTextMarkerRangeFrom(id anchor_textmarker, id focus_textmarker) {
return [attributedInnerText attributedSubstringFromRange:range];
}
- (NSRect)frameForRange:(NSRange)range {
if (!_owner->IsText() && !_owner->IsPlainTextField())
return CGRectNull;
gfx::Rect rect = _owner->GetUnclippedRootFrameInnerTextRangeBoundsRect(
range.location, NSMaxRange(range));
return [self rectInScreen:rect];
}
// Returns the accessibility value for the given attribute. If the value isn't
// supported this will return nil.
- (id)accessibilityAttributeValue:(NSString*)attribute {
@ -3043,13 +3051,8 @@ id content::AXTextMarkerRangeFrom(id anchor_textmarker, id focus_textmarker) {
if ([attribute isEqualToString:
NSAccessibilityBoundsForRangeParameterizedAttribute]) {
if (!_owner->IsText())
return nil;
NSRange range = [(NSValue*)parameter rangeValue];
gfx::Rect rect = _owner->GetUnclippedScreenInnerTextRangeBoundsRect(
range.location, range.location + range.length);
NSRect nsrect = [self rectInScreen:rect];
return [NSValue valueWithRect:nsrect];
NSRect rect = [self frameForRange:[(NSValue*)parameter rangeValue]];
return CGRectIsNull(rect) ? nil : [NSValue valueWithRect:rect];
}
if ([attribute isEqualToString:

@ -151,6 +151,91 @@ IN_PROC_BROWSER_TEST_F(BrowserAccessibilityCocoaBrowserTest,
}
}
IN_PROC_BROWSER_TEST_F(BrowserAccessibilityCocoaBrowserTest,
TestCoordinatesAreInScreenSpace) {
EXPECT_TRUE(NavigateToURL(shell(), GURL(url::kAboutBlankURL)));
AccessibilityNotificationWaiter waiter(shell()->web_contents(),
ui::kAXModeComplete,
ax::mojom::Event::kLoadComplete);
GURL url(R"HTML(data:text/html, <p>Hello, world!</p>)HTML");
EXPECT_TRUE(NavigateToURL(shell(), url));
waiter.WaitForNotification();
BrowserAccessibility* text = FindNode(ax::mojom::Role::kStaticText);
ASSERT_NE(nullptr, text);
BrowserAccessibilityCocoa* cocoa_text = ToBrowserAccessibilityCocoa(text);
ASSERT_NE(nil, cocoa_text);
NSPoint position = [[cocoa_text position] pointValue];
NSSize size = [[cocoa_text size] sizeValue];
NSRect frame = NSMakeRect(position.x, position.y, size.width, size.height);
NSPoint p0_before = position;
NSRect r0_before = [cocoa_text frameForRange:NSMakeRange(0, 5)];
EXPECT_TRUE(CGRectContainsRect(frame, r0_before));
// On macOS geometry accessibility attributes are expected to use the
// screen coordinate system with the origin at the bottom left corner.
// We need some creativity with testing this because it is challenging
// to setup a text element with a precise screen position.
//
// Content shell's window is pinned to have an origin at (0, 0), so
// when its height is changed the content's screen y-coordinate is
// changed by the same amount (see below).
//
// Y^ original
// |
// +--------------------------------------------+
// | |
// | |
// | |
// | |
// h +---------------------------+ |
// | Content Shell | |
// |---------------------------| |
// y |Hello, world | |
// | | |
// | | Screen|
// +---------------------------+----------------+-->
// 0 X
//
// Y^ content shell enlarged
// |
// +--------------------------------------------+
// | |
// | |
// h+dh +---------------------------+ |
// | Content Shell | |
// |---------------------------| |
// y+dh |Hello, world | |
// | | |
// | | |
// | | |
// | | Screen|
// +---------------------------+----------------+-->
// 0 X
//
// This observation allows us to validate the returned
// attribute values and catch the most glaring mistakes
// in coordinate space handling.
const int dh = 100;
gfx::Size content_size = Shell::GetShellDefaultSize();
content_size.Enlarge(0, dh);
shell()->ResizeWebContentForTests(content_size);
NSPoint p0_after = [[cocoa_text position] pointValue];
NSRect r0_after = [cocoa_text frameForRange:NSMakeRange(0, 5)];
ASSERT_EQ(p0_before.y + dh, p0_after.y);
ASSERT_EQ(r0_before.origin.y + dh, r0_after.origin.y);
ASSERT_EQ(r0_before.size.height, r0_after.size.height);
}
IN_PROC_BROWSER_TEST_F(BrowserAccessibilityCocoaBrowserTest,
TestUnlabeledImageRoleDescription) {
ui::AXTreeUpdate tree;

@ -392,6 +392,69 @@ TEST_F(BrowserAccessibilityTest, GetInnerTextRangeBoundsRect) {
#endif
}
TEST_F(BrowserAccessibilityTest, GetInnerTextRangeBoundsRectPlainTextField) {
// Text area with 'Hello' text
// rootWebArea
// ++textField
// ++++genericContainer
// ++++++staticText
// ++++++++inlineTextBox
ui::AXNodeData root;
root.id = 1;
root.role = ax::mojom::Role::kRootWebArea;
root.relative_bounds.bounds = gfx::RectF(0, 0, 800, 600);
ui::AXNodeData textarea;
textarea.id = 2;
textarea.role = ax::mojom::Role::kTextField;
textarea.SetValue("Hello");
textarea.relative_bounds.bounds = gfx::RectF(100, 100, 150, 20);
root.child_ids.push_back(2);
ui::AXNodeData container;
container.id = 3;
container.role = ax::mojom::Role::kGenericContainer;
container.relative_bounds.bounds = textarea.relative_bounds.bounds;
textarea.child_ids.push_back(3);
ui::AXNodeData static_text;
static_text.id = 4;
static_text.role = ax::mojom::Role::kStaticText;
static_text.SetName("Hello");
static_text.relative_bounds.bounds = gfx::RectF(100, 100, 50, 10);
container.child_ids.push_back(4);
ui::AXNodeData inline_text1;
inline_text1.id = 5;
inline_text1.role = ax::mojom::Role::kInlineTextBox;
inline_text1.SetName("Hello");
inline_text1.relative_bounds.bounds = gfx::RectF(100, 100, 50, 10);
inline_text1.AddIntListAttribute(
ax::mojom::IntListAttribute::kCharacterOffsets, {10, 20, 30, 40, 50});
inline_text1.SetTextDirection(ax::mojom::WritingDirection::kLtr);
static_text.child_ids.push_back(5);
std::unique_ptr<BrowserAccessibilityManager> browser_accessibility_manager(
BrowserAccessibilityManager::Create(
MakeAXTreeUpdate(root, textarea, container, static_text,
inline_text1),
test_browser_accessibility_delegate_.get()));
BrowserAccessibility* root_accessible =
browser_accessibility_manager->GetRoot();
ASSERT_NE(nullptr, root_accessible);
BrowserAccessibility* textarea_accessible =
root_accessible->PlatformGetChild(0);
ASSERT_NE(nullptr, textarea_accessible);
// Validate the bounds of 'ell'.
EXPECT_EQ(gfx::Rect(110, 100, 30, 10),
textarea_accessible->GetInnerTextRangeBoundsRect(
1, 4, ui::AXCoordinateSystem::kRootFrame,
ui::AXClippingBehavior::kUnclipped));
}
TEST_F(BrowserAccessibilityTest, GetInnerTextRangeBoundsRectMultiElement) {
ui::AXNodeData root;
root.id = 1;