macOS: Don't include two windows in a11y tree. (#15899)

* Don't include two windows in a11y tree.

`AvnRootAccessibilityElement` has been removed and now `AvnWindow` handles the accessibility protocol itself, exposing its children via the `AvnView`.

* Remove hack now that issue is fixed.

* Fix build errors after merge.

native/Avalonia.Native/src/OSX/Avalonia.Native.OSX.xcodeproj/project.pbxproj

@@ -57,6 +57,7 @@
 		AB8F7D6B21482D7F0057DBA5 /* platformthreading.mm in Sources */ = {isa = PBXBuildFile; fileRef = AB8F7D6A21482D7F0057DBA5 /* platformthreading.mm */; };
 		BC11A5BE2608D58F0017BAD0 /* automation.h in Headers */ = {isa = PBXBuildFile; fileRef = BC11A5BC2608D58F0017BAD0 /* automation.h */; };
 		BC11A5BF2608D58F0017BAD0 /* automation.mm in Sources */ = {isa = PBXBuildFile; fileRef = BC11A5BD2608D58F0017BAD0 /* automation.mm */; };
+		BC7C33822C066DBF00945A48 /* AvnAutomationNode.h in Headers */ = {isa = PBXBuildFile; fileRef = BC7C33812C066DBF00945A48 /* AvnAutomationNode.h */; };
 		ED3791C42862E1F40080BD62 /* UniformTypeIdentifiers.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = ED3791C32862E1F40080BD62 /* UniformTypeIdentifiers.framework */; };
 		ED754D262A97306B0078B4DF /* PlatformRenderTimer.mm in Sources */ = {isa = PBXBuildFile; fileRef = ED754D252A97306B0078B4DF /* PlatformRenderTimer.mm */; };
 		EDF8CDCD2964CB01001EE34F /* PlatformSettings.mm in Sources */ = {isa = PBXBuildFile; fileRef = EDF8CDCC2964CB01001EE34F /* PlatformSettings.mm */; };
@@ -122,6 +123,8 @@
 		AB8F7D6A21482D7F0057DBA5 /* platformthreading.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = platformthreading.mm; sourceTree = "<group>"; };
 		BC11A5BC2608D58F0017BAD0 /* automation.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = automation.h; sourceTree = "<group>"; };
 		BC11A5BD2608D58F0017BAD0 /* automation.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = automation.mm; sourceTree = "<group>"; };
+		BC7C33812C066DBF00945A48 /* AvnAutomationNode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AvnAutomationNode.h; sourceTree = "<group>"; };
+		BC7C33832C066F1100945A48 /* AvnAccessibility.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AvnAccessibility.h; sourceTree = "<group>"; };
 		ED3791C32862E1F40080BD62 /* UniformTypeIdentifiers.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = UniformTypeIdentifiers.framework; path = System/Library/Frameworks/UniformTypeIdentifiers.framework; sourceTree = SDKROOT; };
 		ED754D252A97306B0078B4DF /* PlatformRenderTimer.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = PlatformRenderTimer.mm; sourceTree = "<group>"; };
 		EDF8CDCC2964CB01001EE34F /* PlatformSettings.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = PlatformSettings.mm; sourceTree = "<group>"; };
@@ -167,6 +170,8 @@
 			isa = PBXGroup;
 			children = (
 				F10084852BFF1FB40024303E /* TopLevelImpl.mm */,
+				BC7C33832C066F1100945A48 /* AvnAccessibility.h */,
+				BC7C33812C066DBF00945A48 /* AvnAutomationNode.h */,
 				ED754D252A97306B0078B4DF /* PlatformRenderTimer.mm */,
 				855EDC9E28C6546F00807998 /* PlatformBehaviorInhibition.mm */,
 				8D2F3511292F6AAE007FCF54 /* AvnTextInputMethodDelegate.h */,
@@ -245,6 +250,7 @@
 				183916173528EC2737DBE5E1 /* WindowBaseImpl.h in Headers */,
 				1839171DCC651B0638603AC4 /* INSWindowHolder.h in Headers */,
 				183919D91DB9AAB5D700C2EA /* WindowImpl.h in Headers */,
+				BC7C33822C066DBF00945A48 /* AvnAutomationNode.h in Headers */,
 				18391CF07316F819E76B617C /* IWindowStateChanged.h in Headers */,
 				8D300D65292D0A6800320C49 /* AvnTextInputMethod.h in Headers */,
 				8D2F3512292F6AAE007FCF54 /* AvnTextInputMethodDelegate.h in Headers */,

native/Avalonia.Native/src/OSX/AvnAccessibility.h

@@ -0,0 +1,13 @@
+#pragma once
+#import <Cocoa/Cocoa.h>
+#import "avalonia-native.h"
+
+// Defines the interface between AvnAutomationNode and objects which implement
+// NSAccessibility such as AvnAccessibilityElement or AvnWindow.
+@protocol AvnAccessibility <NSAccessibility>
+@required
+- (void) raiseChildrenChanged;
+@optional
+- (void) raiseFocusChanged;
+- (void) raisePropertyChanged:(AvnAutomationProperty)property;
+@end

native/Avalonia.Native/src/OSX/AvnAutomationNode.h

@@ -0,0 +1,18 @@
+#pragma once
+#include "avalonia-native.h"
+#include "AvnAccessibility.h"
+
+// Defines a means for managed code to raise accessibility events.
+class AvnAutomationNode : public ComSingleObject<IAvnAutomationNode, &IID_IAvnAutomationNode>
+{
+public:
+    FORWARD_IUNKNOWN()
+    AvnAutomationNode(id <AvnAccessibility> owner) { _owner = owner; }
+    AvnAccessibilityElement* GetOwner() { return _owner; }
+    virtual void Dispose() override { _owner = nil; }
+    virtual void ChildrenChanged () override { [_owner raiseChildrenChanged]; }
+    virtual void PropertyChanged (AvnAutomationProperty property) override { [_owner raisePropertyChanged:property]; }
+    virtual void FocusChanged () override { [_owner raiseFocusChanged]; }
+private:
+    __strong id <AvnAccessibility> _owner;
+};

native/Avalonia.Native/src/OSX/AvnView.h

@@ -22,5 +22,6 @@
 -(AvnPlatformResizeReason) getResizeReason;
 -(void) setResizeReason:(AvnPlatformResizeReason)reason;
 -(void) setRenderTarget:(NSObject<IRenderTarget>* _Nonnull)target;
+-(void) raiseAccessibilityChildrenChanged;
 + (AvnPoint)toAvnPoint:(CGPoint)p;
 @end

native/Avalonia.Native/src/OSX/AvnView.mm

@@ -19,12 +19,12 @@
     AvnPixelSize _lastPixelSize;
     NSObject<IRenderTarget>* _currentRenderTarget;
     AvnPlatformResizeReason _resizeReason;
-    AvnAccessibilityElement* _accessibilityChild;
     NSRect _cursorRect;
     NSMutableAttributedString* _text;
     NSRange _selectedRange;
     NSRange _markedRange;
     NSEvent* _lastKeyDownEvent;
+    NSMutableArray* _accessibilityChildren;
 }
 
 - (void)onClosed
@@ -801,35 +801,74 @@
     _resizeReason = reason;
 }
 
-- (AvnAccessibilityElement *) accessibilityChild
+- (NSArray *)accessibilityChildren
 {
-    if (_accessibilityChild == nil)
+    if (_accessibilityChildren == nil)
-    {
+        [self recalculateAccessibiltyChildren];
-        auto peer = _parent->TopLevelEvents->GetAutomationPeer();
+    return _accessibilityChildren;
+}
 
-        if (peer == nil)
+- (id _Nullable) accessibilityHitTest:(NSPoint)point
-            return nil;
+{
+    if (![[self window] isKindOfClass:[AvnWindow class]])
+        return self;
 
-        _accessibilityChild = [AvnAccessibilityElement acquire:peer];
+    auto window = (AvnWindow*)[self window];
-    }
+    auto peer = [window automationPeer];
 
-    return _accessibilityChild;
+    if (!peer->IsRootProvider())
-}
+        return nil;
 
-- (NSArray *)accessibilityChildren
+    auto clientPoint = [window convertPointFromScreen:point];
-{
+    auto localPoint = [self translateLocalPoint:ToAvnPoint(clientPoint)];
-    auto child = [self accessibilityChild];
+    auto hit = peer->RootProvider_GetPeerFromPoint(localPoint);
-    return NSAccessibilityUnignoredChildrenForOnlyChild(child);
+    return [AvnAccessibilityElement acquire:hit];
 }
 
-- (id)accessibilityHitTest:(NSPoint)point
+- (void)raiseAccessibilityChildrenChanged
 {
-    return [[self accessibilityChild] accessibilityHitTest:point];
+    auto changed = _accessibilityChildren ? [NSMutableSet setWithArray:_accessibilityChildren] : [NSMutableSet set];
+
+    [self recalculateAccessibiltyChildren];
+
+    if (_accessibilityChildren)
+        [changed addObjectsFromArray:_accessibilityChildren];
+
+    NSAccessibilityPostNotificationWithUserInfo(
+        self,
+        NSAccessibilityLayoutChangedNotification,
+        @{ NSAccessibilityUIElementsKey: [changed allObjects]});
 }
 
-- (id)accessibilityFocusedUIElement
+- (void)recalculateAccessibiltyChildren
 {
-    return [[self accessibilityChild] accessibilityFocusedUIElement];
+    _accessibilityChildren = [[NSMutableArray alloc] init];
+
+    if (![[self window] isKindOfClass:[AvnWindow class]])
+    {
+        return;
+    }
+
+    // The accessibility children of the Window are exposed as children
+    // of the AvnView.
+    auto window = (AvnWindow*)[self window];
+    auto peer = [window automationPeer];
+    auto childPeers = peer->GetChildren();
+    auto childCount = childPeers != nullptr ? childPeers->GetCount() : 0;
+
+    if (childCount > 0)
+    {
+        for (int i = 0; i < childCount; ++i)
+        {
+            IAvnAutomationPeer* child;
+
+            if (childPeers->Get(i, &child) == S_OK)
+            {
+                id element = [AvnAccessibilityElement acquire:child];
+                [_accessibilityChildren addObject:element];
+            }
+        }
+    }
 }
 
 - (void) setText:(NSString *)text{

native/Avalonia.Native/src/OSX/AvnWindow.mm

@@ -24,6 +24,8 @@
 #include "WindowImpl.h"
 #include "AvnView.h"
 #include "WindowInterfaces.h"
+#include "AvnAutomationNode.h"
+#include "AvnString.h"
 
 @implementation CLASS_NAME
 {
@@ -34,6 +36,13 @@
     bool _isExtended;
     bool _isTransitioningToFullScreen;
     AvnMenu* _menu;
+    IAvnAutomationPeer* _automationPeer;
+    AvnAutomationNode* _automationNode;
+}
+
+-(AvnView* _Nullable) view
+{
+    return _parent->View;
 }
 
 -(void) setIsExtended:(bool)value;
@@ -208,7 +217,7 @@
         ComPtr<WindowBaseImpl> parent = _parent;
         _parent = NULL;
         
-        auto window = dynamic_cast<WindowImpl*>(parent.getRaw());
+            auto window = dynamic_cast<WindowImpl*>(parent.getRaw());
         
         if(window != nullptr)
         {
@@ -489,5 +498,41 @@
     _parent = nullptr;
 }
 
+- (id _Nullable) accessibilityFocusedUIElement
+{
+    if (![self automationPeer]->IsRootProvider())
+        return nil;
+    auto focusedPeer = [self automationPeer]->RootProvider_GetFocus();
+    return [AvnAccessibilityElement acquire:focusedPeer];
+}
+
+- (NSString * _Nullable) accessibilityIdentifier
+{
+    return GetNSStringAndRelease([self automationPeer]->GetAutomationId());
+}
+
+- (IAvnAutomationPeer* _Nonnull) automationPeer
+{
+    if (_automationPeer == nullptr)
+    {
+        _automationPeer = _parent->BaseEvents->GetAutomationPeer();
+        _automationNode = new AvnAutomationNode(self);
+        _automationPeer->SetNode(_automationNode);
+    }
+
+    return _automationPeer;
+}
+
+- (void)raiseChildrenChanged
+{
+    [_parent->View raiseAccessibilityChildrenChanged];
+}
+
+- (void)raiseFocusChanged
+{
+    id focused = [self accessibilityFocusedUIElement];
+    NSAccessibilityPostNotification(focused, NSAccessibilityFocusedUIElementChangedNotification);
+}
+
 @end
 

native/Avalonia.Native/src/OSX/WindowInterfaces.h

@@ -7,11 +7,13 @@
 #import <AppKit/AppKit.h>
 #include "WindowProtocol.h"
 #include "WindowBaseImpl.h"
+#include "AvnAccessibility.h"
 
-@interface AvnWindow : NSWindow <AvnWindowProtocol, NSWindowDelegate>
+@interface AvnWindow : NSWindow <AvnWindowProtocol, NSWindowDelegate, AvnAccessibility>
 -(AvnWindow* _Nonnull) initWithParent: (WindowBaseImpl* _Nonnull) parent contentRect: (NSRect)contentRect styleMask: (NSWindowStyleMask)styleMask;
+-(AvnView* _Nullable) view;
 @end
 
-@interface AvnPanel : NSPanel <AvnWindowProtocol, NSWindowDelegate>
+@interface AvnPanel : NSPanel <AvnWindowProtocol, NSWindowDelegate, AvnAccessibility>
 -(AvnPanel* _Nonnull) initWithParent: (WindowBaseImpl* _Nonnull) parent contentRect: (NSRect)contentRect styleMask: (NSWindowStyleMask)styleMask;
-@end
+@end

native/Avalonia.Native/src/OSX/WindowProtocol.h

@@ -8,6 +8,7 @@
 #import <AppKit/AppKit.h>
 
 @class AvnMenu;
+struct IAvnAutomationPeer;
 
 @protocol AvnWindowProtocol
 -(void) pollModalSession: (NSModalSession _Nonnull) session;
@@ -16,6 +17,7 @@
 -(void) showAppMenuOnly;
 -(void) showWindowMenuWithAppMenu;
 -(void) applyMenu:(AvnMenu* _Nullable)menu;
+-(IAvnAutomationPeer* _Nonnull) automationPeer;
 
 -(double) getExtendedTitleBarHeight;
 -(void) setIsExtended:(bool)value;

native/Avalonia.Native/src/OSX/automation.h

@@ -1,12 +1,13 @@
 #pragma once
 
 #import <Cocoa/Cocoa.h>
+#include "AvnAccessibility.h"
 NS_ASSUME_NONNULL_BEGIN
 
 class IAvnAutomationPeer;
 
-@interface AvnAccessibilityElement : NSAccessibilityElement
+@interface AvnAccessibilityElement : NSAccessibilityElement <AvnAccessibility>
-+ (AvnAccessibilityElement *) acquire:(IAvnAutomationPeer *) peer;
++ (id _Nullable) acquire:(IAvnAutomationPeer *) peer;
 @end
 
 NS_ASSUME_NONNULL_END

native/Avalonia.Native/src/OSX/automation.mm

@@ -1,66 +1,19 @@
 #include "common.h"
 #include "automation.h"
+#include "AvnAutomationNode.h"
 #include "AvnString.h"
 #include "INSWindowHolder.h"
 #include "AvnView.h"
-
+#include "WindowInterfaces.h"
-@interface AvnAccessibilityElement (Events)
-- (void) raiseChildrenChanged;
-@end
-
-@interface AvnRootAccessibilityElement : AvnAccessibilityElement
-- (AvnView *) ownerView;
-- (AvnRootAccessibilityElement *) initWithPeer:(IAvnAutomationPeer *) peer owner:(AvnView*) owner;
-- (void) raiseFocusChanged;
-@end
-
-class AutomationNode : public ComSingleObject<IAvnAutomationNode, &IID_IAvnAutomationNode>
-{
-public:
-    FORWARD_IUNKNOWN()
-
-    AutomationNode(AvnAccessibilityElement* owner)
-    {
-        _owner = owner;
-    }
-    
-    AvnAccessibilityElement* GetOwner()
-    {
-        return _owner;
-    }
-    
-    virtual void Dispose() override
-    {
-        _owner = nil;
-    }
-    
-    virtual void ChildrenChanged () override
-    {
-        [_owner raiseChildrenChanged];
-    }
-    
-    virtual void PropertyChanged (AvnAutomationProperty property) override
-    {
-        
-    }
-    
-    virtual void FocusChanged () override
-    {
-        [(AvnRootAccessibilityElement*)_owner raiseFocusChanged];
-    }
-    
-private:
-    __strong AvnAccessibilityElement* _owner;
-};
 
 @implementation AvnAccessibilityElement
 {
     IAvnAutomationPeer* _peer;
-    AutomationNode* _node;
+    AvnAutomationNode* _node;
     NSMutableArray* _children;
 }
 
-+ (AvnAccessibilityElement *)acquire:(IAvnAutomationPeer *)peer
++ (id _Nullable)acquire:(IAvnAutomationPeer *)peer
 {
     if (peer == nullptr)
         return nil;
@@ -68,7 +21,7 @@ private:
     auto instance = peer->GetNode();
     
     if (instance != nullptr)
-        return dynamic_cast<AutomationNode*>(instance)->GetOwner();
+        return dynamic_cast<AvnAutomationNode*>(instance)->GetOwner();
     
     if (peer->IsRootProvider())
     {
@@ -82,7 +35,7 @@ private:
         
         auto holder = dynamic_cast<INSViewHolder*>(window);
         auto view = holder->GetNSView();
-        return [[AvnRootAccessibilityElement alloc] initWithPeer:peer owner:view];
+        return [view window];
     }
     else
     {
@@ -94,7 +47,7 @@ private:
 {
     self = [super init];
     _peer = peer;
-    _node = new AutomationNode(self);
+    _node = new AvnAutomationNode(self);
     _peer->SetNode(_node);
     return self;
 }
@@ -256,25 +209,8 @@ private:
 
 - (NSRect)accessibilityFrame
 {
-    id topLevel = [self accessibilityTopLevelUIElement];
+    auto bounds = _peer->GetBoundingRectangle();
-    auto result = NSZeroRect;
+    return [self rectToScreen:bounds];
-
-    if ([topLevel isKindOfClass:[AvnRootAccessibilityElement class]])
-    {
-        auto root = (AvnRootAccessibilityElement*)topLevel;
-        auto view = [root ownerView];
-        
-        if (view)
-        {
-            auto window = [view window];
-            auto bounds = ToNSRect(_peer->GetBoundingRectangle());
-            auto windowBounds = [view convertRect:bounds toView:nil];
-            auto screenBounds = [window convertRectToScreen:windowBounds];
-            result = screenBounds;
-        }
-    }
-
-    return result;
 }
 
 - (id)accessibilityParent
@@ -389,6 +325,24 @@ private:
     return [super isAccessibilitySelectorAllowed:selector];
 }
 
+- (NSRect)rectToScreen:(AvnRect)rect
+{
+    id topLevel = [self accessibilityTopLevelUIElement];
+
+    if (![topLevel isKindOfClass:[AvnWindow class]])
+        return NSZeroRect;
+
+    auto window = (AvnWindow*)topLevel;
+    auto view = [window view];
+
+    if (view == nil)
+        return NSZeroRect;
+
+    auto nsRect = ToNSRect(rect);
+    auto windowRect = [view convertRect:nsRect toView:nil];
+    return [window convertRectToScreen:windowRect];
+}
+
 - (void)raiseChildrenChanged
 {
     auto changed = _children ? [NSMutableSet setWithArray:_children] : [NSMutableSet set];
@@ -429,7 +383,7 @@ private:
             
             if (childPeers->Get(i, &child) == S_OK)
             {
-                auto element = [AvnAccessibilityElement acquire:child];
+                id element = [AvnAccessibilityElement acquire:child];
                 [_children addObject:element];
             }
         }
@@ -441,64 +395,3 @@ private:
 }
 
 @end
-
-@implementation AvnRootAccessibilityElement
-{
-    AvnView* _owner;
-}
-
-- (AvnRootAccessibilityElement *)initWithPeer:(IAvnAutomationPeer *)peer owner:(AvnView *)owner
-{
-    self = [super initWithPeer:peer];
-    _owner = owner;
-
-    // Seems we need to raise a focus changed notification here if we have focus
-    auto focusedPeer = [self peer]->RootProvider_GetFocus();
-    id focused = [AvnAccessibilityElement acquire:focusedPeer];
-
-    if (focused)
-        NSAccessibilityPostNotification(focused, NSAccessibilityFocusedUIElementChangedNotification);
-    
-    return self;
-}
-
-- (AvnView *)ownerView
-{
-    return _owner;
-}
-
-- (id)accessibilityFocusedUIElement
-{
-    auto focusedPeer = [self peer]->RootProvider_GetFocus();
-    return [AvnAccessibilityElement acquire:focusedPeer];
-}
-
-- (id)accessibilityHitTest:(NSPoint)point
-{
-    auto clientPoint = [[_owner window] convertPointFromScreen:point];
-    auto localPoint = [_owner translateLocalPoint:ToAvnPoint(clientPoint)];
-    auto hit = [self peer]->RootProvider_GetPeerFromPoint(localPoint);
-    return [AvnAccessibilityElement acquire:hit];
-}
-
-- (id)accessibilityParent
-{
-    return _owner;
-}
-
-- (void)raiseFocusChanged
-{
-    id focused = [self accessibilityFocusedUIElement];
-    NSAccessibilityPostNotification(focused, NSAccessibilityFocusedUIElementChangedNotification);
-}
-
-// Although this method is marked as deprecated we get runtime warnings if we don't handle it.
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-implementations"
-- (void)accessibilityPerformAction:(NSAccessibilityActionName)action
-{
-    [_owner accessibilityPerformAction:action];
-}
-#pragma clang diagnostic pop
-
-@end

tests/Avalonia.IntegrationTests.Appium/WindowTests.cs

@@ -417,10 +417,8 @@ namespace Avalonia.IntegrationTests.Appium
         {
             if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
             {
-                // The Avalonia a11y tree currently exposes two nested Window elements, this is a bug and should be fixed 
-                // but in the meantime use the `parent::' selector to return the parent "real" window. 
                 return _session.FindElementByXPath(
-                    $"XCUIElementTypeWindow//*[@identifier='{identifier}']/parent::XCUIElementTypeWindow");
+                    $"XCUIElementTypeWindow[@identifier='{identifier}']");
             }
             else
             {

tests/Avalonia.IntegrationTests.Appium/WindowTests_MacOS.cs

@@ -448,10 +448,7 @@ namespace Avalonia.IntegrationTests.Appium
 
         private AppiumWebElement GetWindow(string identifier)
         {
-            // The Avalonia a11y tree currently exposes two nested Window elements, this is a bug and should be fixed 
+            return _session.FindElementByXPath($"XCUIElementTypeWindow[@identifier='{identifier}']");
-            // but in the meantime use the `parent::' selector to return the parent "real" window. 
-            return _session.FindElementByXPath(
-                $"XCUIElementTypeWindow//*[@identifier='{identifier}']/parent::XCUIElementTypeWindow");
         }
 
         private int GetWindowOrder(string identifier)