From 5484f89eb58e020d0b4ee42a95281855f5070b6a Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Wed, 1 Mar 2023 16:04:44 -0800 Subject: [PATCH 1/2] [macOS] Remove single accessibility root assumptions --- .../Source/AccessibilityBridgeMac.mm | 4 +- .../Source/AccessibilityBridgeMacTest.mm | 83 ++++++++++++++++++- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.mm index d124bcfe34826..be715db20c99f 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.mm +++ b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.mm @@ -187,9 +187,7 @@ if (ax_node.data().HasState(ax::mojom::State::kEditable)) { events.push_back({ .name = NSAccessibilityValueChangedNotification, - .target = GetFlutterPlatformNodeDelegateFromID(AccessibilityBridge::kRootNodeId) - .lock() - ->GetNativeViewAccessible(), + .target = RootDelegate()->GetNativeViewAccessible(), .user_info = nil, }); } diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm index c13335dd63033..fdb11ece0a6c8 100644 --- a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm +++ b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacTest.mm @@ -58,7 +58,7 @@ @implementation AccessibilityBridgeTestViewController } } // namespace -TEST(AccessibilityBridgeMacTest, sendsAccessibilityCreateNotificationToWindowOfFlutterView) { +TEST(AccessibilityBridgeMacTest, SendsAccessibilityCreateNotificationToWindowOfFlutterView) { FlutterViewController* viewController = CreateTestViewController(); FlutterEngine* engine = viewController.engine; [viewController loadView]; @@ -112,7 +112,84 @@ @implementation AccessibilityBridgeTestViewController [engine shutDownEngine]; } -TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenHeadless) { +// Flutter used to assume that the accessibility root had ID 0. +// In a multi-view world, each view has its own accessibility root +// with a globally unique node ID. +// +// node1 +// | +// node2 +TEST(AccessibilityBridgeMacTest, NonZeroRootNodeId) { + FlutterViewController* viewController = CreateTestViewController(); + FlutterEngine* engine = viewController.engine; + [viewController loadView]; + + NSWindow* expectedTarget = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) + styleMask:NSBorderlessWindowMask + backing:NSBackingStoreBuffered + defer:NO]; + expectedTarget.contentView = viewController.view; + // Setting up bridge so that the AccessibilityBridgeMacDelegateSpy + // can query semantics information from. + engine.semanticsEnabled = YES; + auto bridge = std::static_pointer_cast( + viewController.accessibilityBridge.lock()); + + FlutterSemanticsNode node1; + std::vector node1_children{2}; + node1.id = 1; + node1.flags = static_cast(0); + node1.actions = static_cast(0); + node1.text_selection_base = -1; + node1.text_selection_extent = -1; + node1.label = "node1"; + node1.hint = ""; + node1.value = ""; + node1.increased_value = ""; + node1.decreased_value = ""; + node1.tooltip = ""; + node1.child_count = node1_children.size(); + node1.children_in_traversal_order = node1_children.data(); + node1.children_in_hit_test_order = node1_children.data(); + node1.custom_accessibility_actions_count = 0; + + FlutterSemanticsNode node2; + node2.id = 2; + node2.flags = static_cast(0); + node2.actions = static_cast(0); + node2.text_selection_base = -1; + node2.text_selection_extent = -1; + node2.label = "node2"; + node2.hint = ""; + node2.value = ""; + node2.increased_value = ""; + node2.decreased_value = ""; + node2.tooltip = ""; + node2.child_count = 0; + node2.custom_accessibility_actions_count = 0; + + bridge->AddFlutterSemanticsNodeUpdate(node1); + bridge->AddFlutterSemanticsNodeUpdate(node2); + bridge->CommitUpdates(); + + // Look up the root node delegate. + auto root_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock(); + ASSERT_TRUE(root_delegate); + ASSERT_EQ(root_delegate->GetChildCount(), 1); + + // Look up the child node delegate. + auto child_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(2).lock(); + ASSERT_TRUE(child_delegate); + ASSERT_EQ(child_delegate->GetChildCount(), 0); + + // Ensure a node with ID 0 does not exist. + auto invalid_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock(); + ASSERT_FALSE(invalid_delegate); + + [engine shutDownEngine]; +} + +TEST(AccessibilityBridgeMacTest, DoesNotSendAccessibilityCreateNotificationWhenHeadless) { FlutterViewController* viewController = CreateTestViewController(); FlutterEngine* engine = viewController.engine; [viewController loadView]; @@ -158,7 +235,7 @@ @implementation AccessibilityBridgeTestViewController [engine shutDownEngine]; } -TEST(AccessibilityBridgeMacTest, doesNotSendAccessibilityCreateNotificationWhenNoWindow) { +TEST(AccessibilityBridgeMacTest, DoesNotSendAccessibilityCreateNotificationWhenNoWindow) { FlutterViewController* viewController = CreateTestViewController(); FlutterEngine* engine = viewController.engine; [viewController loadView]; From 57de4d69e742429a3dd996c04b42d7ae3d3a144b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Sharma?= Date: Thu, 16 Mar 2023 16:14:15 -0700 Subject: [PATCH 2/2] Remove deprecated root node ID constant --- shell/platform/common/accessibility_bridge.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/shell/platform/common/accessibility_bridge.h b/shell/platform/common/accessibility_bridge.h index b654dad66f8bb..bc9567624f4b1 100644 --- a/shell/platform/common/accessibility_bridge.h +++ b/shell/platform/common/accessibility_bridge.h @@ -48,13 +48,6 @@ class AccessibilityBridge AccessibilityBridge(); virtual ~AccessibilityBridge(); - //----------------------------------------------------------------------------- - /// @brief The ID of the root node in the accessibility tree. In Flutter, - /// this is always 0. - // TODO(loicsharma): Remove this as it is incorrect in a multi-view world. - // See: https://github.com/flutter/flutter/issues/119391 - static constexpr int32_t kRootNodeId = 0; - //------------------------------------------------------------------------------ /// @brief Adds a semantics node update to the pending semantics update. /// Calling this method alone will NOT update the semantics tree.