From c9d2d58ae9f88ffffa8d8766de4b9527de238c79 Mon Sep 17 00:00:00 2001 From: Valerie Young Date: Wed, 20 Jul 2022 17:40:45 +0000 Subject: [PATCH] Revert "Reland: "mac_merge: remove BrowserAccessibilityCocoa accessibilityFrame in favor of AXPlatformNodeCocoa"" This reverts commit 32a8a89d417639b7212319d895eeb7130311b068. Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=1345985 Original change's description: > Reland: "mac_merge: remove BrowserAccessibilityCocoa accessibilityFrame in favor of AXPlatformNodeCocoa" > > This is a reland of commit I0a4464d3c74f52e9bd290aa8a70047ff2cd51cb5 > > In the previous commit, the test of accessibilityFrame API caused a failure on the Mac 10.13 bot: > https://chromium-review.googlesource.com/c/chromium/src/+/3687274 > > These changes now filter out the "position" from the test of the API in order to prevent test flakiness. In order to do this, support for > NSRect has been added to the @MAC-SCRIPT test framework. > > Original description: > > mac_merge: remove BrowserAccessibilityCocoa accessibilityFrame in favor of AXPlatformNodeCocoa > > > > Bug: 939860 > > Change-Id: I4509c45f95ee16635208cb0368fae81f73347402 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3580034 > > Reviewed-by: Nektarios Paisios > > Commit-Queue: Valerie Young > > Cr-Commit-Position: refs/heads/main@{#1010359} > > Bug: 939860 > Change-Id: I726abfd86ef1890e588b638b43a318bf82824d40 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3698783 > Commit-Queue: Valerie Young > Reviewed-by: Nektarios Paisios > Cr-Commit-Position: refs/heads/main@{#1024762} Bug: 939860 Change-Id: I6bebd18c77edf4dd1bf71b83e278cc9d884a3389 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3777323 Bot-Commit: Rubber Stamper Reviewed-by: Aaron Leventhal Commit-Queue: Valerie Young Cr-Commit-Position: refs/heads/main@{#1026317} --- .../browser_accessibility_cocoa.mm | 15 +++++++++++ .../dump_accessibility_scripts_browsertest.cc | 4 --- .../methods/accessibility-frame-expected.txt | 4 --- .../mac/methods/accessibility-frame.html | 14 ---------- .../platform/ax_platform_node_cocoa.h | 1 + .../platform/ax_platform_node_cocoa.mm | 18 ++++++++----- ui/accessibility/platform/ax_utils_mac.h | 2 -- ui/accessibility/platform/ax_utils_mac.mm | 8 ------ .../inspect/ax_call_statement_invoker_mac.h | 4 --- .../inspect/ax_call_statement_invoker_mac.mm | 26 ------------------- 10 files changed, 27 insertions(+), 69 deletions(-) delete mode 100644 content/test/data/accessibility/mac/methods/accessibility-frame-expected.txt delete mode 100644 content/test/data/accessibility/mac/methods/accessibility-frame.html diff --git a/content/browser/accessibility/browser_accessibility_cocoa.mm b/content/browser/accessibility/browser_accessibility_cocoa.mm index 27d8447fdd029f..abe2fb9541393e 100644 --- a/content/browser/accessibility/browser_accessibility_cocoa.mm +++ b/content/browser/accessibility/browser_accessibility_cocoa.mm @@ -2645,6 +2645,21 @@ - (BOOL)isAccessibilityEnabled { ax::mojom::Restriction::kDisabled; } +- (NSRect)accessibilityFrame { + if (![self instanceActive]) + return NSZeroRect; + + BrowserAccessibilityManager* manager = _owner->manager(); + auto rect = _owner->GetBoundsRect(ui::AXCoordinateSystem::kScreenDIPs, + ui::AXClippingBehavior::kClipped); + + // TODO(vmpstr): GetBoundsRect() call above should account for this instead. + auto result_rect = + ScaleToRoundedRect(rect, 1.f / manager->device_scale_factor()); + + return gfx::ScreenRectToNSRect(result_rect); +} + - (BOOL)isCheckable { if (![self instanceActive]) return NO; diff --git a/content/browser/accessibility/dump_accessibility_scripts_browsertest.cc b/content/browser/accessibility/dump_accessibility_scripts_browsertest.cc index 92c1d9a73ec4a6..e526bcb3349ad0 100644 --- a/content/browser/accessibility/dump_accessibility_scripts_browsertest.cc +++ b/content/browser/accessibility/dump_accessibility_scripts_browsertest.cc @@ -452,10 +452,6 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityScriptTest, RunTypedTest("accessibility-column-header-ui-elements.html"); } -IN_PROC_BROWSER_TEST_P(DumpAccessibilityScriptTest, AccessibilityFrame) { - RunTypedTest("accessibility-frame.html"); -} - IN_PROC_BROWSER_TEST_P(DumpAccessibilityScriptTest, AccessibilityIsIgnored) { RunTypedTest("accessibility-is-ignored.html"); } diff --git a/content/test/data/accessibility/mac/methods/accessibility-frame-expected.txt b/content/test/data/accessibility/mac/methods/accessibility-frame-expected.txt deleted file mode 100644 index 3eca3d7de399e3..00000000000000 --- a/content/test/data/accessibility/mac/methods/accessibility-frame-expected.txt +++ /dev/null @@ -1,4 +0,0 @@ -// Only test the size attribute of accessibility frame because testing the -// position results in flakey tests. -div.accessibilityFrame.size={w: 85, h: 18} -clipped.accessibilityFrame.size={w: 80, h: 36} diff --git a/content/test/data/accessibility/mac/methods/accessibility-frame.html b/content/test/data/accessibility/mac/methods/accessibility-frame.html deleted file mode 100644 index 1deeb7507ed3ea..00000000000000 --- a/content/test/data/accessibility/mac/methods/accessibility-frame.html +++ /dev/null @@ -1,14 +0,0 @@ - - -
- This is a test. -
-
- This is a clipped test. -
diff --git a/ui/accessibility/platform/ax_platform_node_cocoa.h b/ui/accessibility/platform/ax_platform_node_cocoa.h index 07f5bedd170379..b6b32b68bc071d 100644 --- a/ui/accessibility/platform/ax_platform_node_cocoa.h +++ b/ui/accessibility/platform/ax_platform_node_cocoa.h @@ -62,6 +62,7 @@ AX_EXPORT - (instancetype)initWithNode:(ui::AXPlatformNodeBase*)node; - (void)detach; +@property(nonatomic, readonly) NSRect boundsInScreen; @property(nonatomic, readonly) ui::AXPlatformNodeBase* node; @property(nonatomic, readonly) ui::AXPlatformNodeDelegate* nodeDelegate; diff --git a/ui/accessibility/platform/ax_platform_node_cocoa.mm b/ui/accessibility/platform/ax_platform_node_cocoa.mm index f774d14abd4e4f..426b6ce73e2e3b 100644 --- a/ui/accessibility/platform/ax_platform_node_cocoa.mm +++ b/ui/accessibility/platform/ax_platform_node_cocoa.mm @@ -652,6 +652,13 @@ - (void)detach { self, NSAccessibilityUIElementDestroyedNotification); } +- (NSRect)boundsInScreen { + if (!_node || !_node->GetDelegate()) + return NSZeroRect; + return gfx::ScreenRectToNSRect(_node->GetDelegate()->GetBoundsRect( + ui::AXCoordinateSystem::kScreenDIPs, ui::AXClippingBehavior::kClipped)); +} + - (NSString*)getStringAttribute:(ax::mojom::StringAttribute)attribute { std::string attributeValue; if (_node->GetStringAttribute(attribute, &attributeValue)) @@ -763,7 +770,7 @@ - (BOOL)accessibilityIsIgnored { } - (id)accessibilityHitTest:(NSPoint)point { - if (!NSPointInRect(point, [self accessibilityFrame])) + if (!NSPointInRect(point, [self boundsInScreen])) return nil; for (id child in [[self AXChildren] reverseObjectEnumerator]) { @@ -1572,11 +1579,11 @@ - (id)AXTopLevelUIElement { } - (NSValue*)AXPosition { - return [NSValue valueWithPoint:self.accessibilityFrame.origin]; + return [NSValue valueWithPoint:self.boundsInScreen.origin]; } - (NSValue*)AXSize { - return [NSValue valueWithSize:self.accessibilityFrame.size]; + return [NSValue valueWithSize:self.boundsInScreen.size]; } - (NSString*)AXTitle { @@ -1761,10 +1768,7 @@ - (BOOL)isAccessibilityEnabled { } - (NSRect)accessibilityFrame { - if (!_node || !_node->GetDelegate()) - return NSZeroRect; - return gfx::ScreenRectToNSRect(_node->GetDelegate()->GetBoundsRect( - ui::AXCoordinateSystem::kScreenDIPs, ui::AXClippingBehavior::kClipped)); + return [self boundsInScreen]; } - (NSString*)accessibilityLabel { diff --git a/ui/accessibility/platform/ax_utils_mac.h b/ui/accessibility/platform/ax_utils_mac.h index 419d8a2522e241..eeea71cf31de5e 100644 --- a/ui/accessibility/platform/ax_utils_mac.h +++ b/ui/accessibility/platform/ax_utils_mac.h @@ -14,8 +14,6 @@ namespace ui { class AXPlatformNodeCocoa; -AX_EXPORT bool IsNSRect(id object); - // An AXTextMarker is used by applications like Chrome to store a position in // the accessibility tree's text representation. It is a data structure whose // contents are opaque to the system but whose allocation and deallocation is diff --git a/ui/accessibility/platform/ax_utils_mac.mm b/ui/accessibility/platform/ax_utils_mac.mm index 918936fe7c035e..02613f02dd3605 100644 --- a/ui/accessibility/platform/ax_utils_mac.mm +++ b/ui/accessibility/platform/ax_utils_mac.mm @@ -38,14 +38,6 @@ AXTextMarkerRangeRef AXTextMarkerRangeCreate(CFAllocatorRef, namespace ui { -bool IsNSRect(id object) { - if (object == nil || ![object isKindOfClass:[NSValue class]]) - return false; - if (0 == strcmp([object objCType], @encode(NSRect))) - return true; - return false; -} - bool IsAXTextMarker(id object) { if (object == nil) return false; diff --git a/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.h b/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.h index bce9b655e168ae..c42f3f4e77d624 100644 --- a/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.h +++ b/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.h @@ -63,10 +63,6 @@ class AX_EXPORT AXCallStatementInvoker final { const id target, const AXPropertyNode& property_node) const; - // Invokes a property node for a given NSRect. - AXOptionalNSObject InvokeForRect(const id target, - const AXPropertyNode& property_node) const; - // Invokes setAccessibilityFocused method. AXOptionalNSObject InvokeSetAccessibilityFocused( const id target, diff --git a/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.mm b/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.mm index eb59fcf8041217..060fad61719c5c 100644 --- a/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.mm +++ b/ui/accessibility/platform/inspect/ax_call_statement_invoker_mac.mm @@ -159,10 +159,6 @@ return InvokeForAXTextMarkerRange(target, property_node); } - if (IsNSRect(target)) { - return InvokeForRect(target, property_node); - } - if ([target isKindOfClass:[NSArray class]]) return InvokeForArray(target, property_node); @@ -402,28 +398,6 @@ return AXOptionalNSObject::NotNullOrError(dictionary[key]); } -AXOptionalNSObject AXCallStatementInvoker::InvokeForRect( - const id target, - const AXPropertyNode& property_node) const { - NSValue* value = target; - NSRect rect = value.rectValue; - - if (property_node.name_or_value == "size") { - NSValue* size = [NSValue valueWithSize:rect.size]; - return AXOptionalNSObject(size); - } - - if (property_node.name_or_value == "position") { - NSValue* position = [NSValue valueWithPoint:rect.origin]; - return AXOptionalNSObject(position); - } - - LOG(ERROR) << "Unrecognized '" << property_node.name_or_value - << "' attribute called on NSRect in '" - << property_node.ToFlatString() << "' statement"; - return AXOptionalNSObject::Error(); -} - AXOptionalNSObject AXCallStatementInvoker::InvokeSetAccessibilityFocused( const id target, const AXPropertyNode& property_node) const {