From 99e1a0652c2a2d0b692d9d02e0f4263e1af1a3d6 Mon Sep 17 00:00:00 2001 From: LouiseHsu Date: Tue, 25 Feb 2025 16:59:54 -0800 Subject: [PATCH] Fix extra numbers showing up when enabling VoiceControl (#163593) Fixes https://github.com/flutter/flutter/issues/158477 and https://github.com/flutter/flutter/issues/156368. The excess numbers in both PRs are caused by all `SemanticObjects` returning `YES` for `accessibilityRespondsToUserInteraction`, even if it has no semantic actions. For example, a SemanticObject with just a label has semantic information (the label) but no action. This PR adds a check, ensuring that an `SemanticObjects` has at least one accessible action before returning `YES` ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. --- .../flutter/lib/ui/semantics/semantics_node.h | 11 +- .../ios/framework/Source/SemanticsObject.mm | 13 ++ .../Source/accessibility_bridge_test.mm | 156 ++++++++++++++++++ 3 files changed, 177 insertions(+), 3 deletions(-) diff --git a/engine/src/flutter/lib/ui/semantics/semantics_node.h b/engine/src/flutter/lib/ui/semantics/semantics_node.h index 4d04d3d94e..5473240d87 100644 --- a/engine/src/flutter/lib/ui/semantics/semantics_node.h +++ b/engine/src/flutter/lib/ui/semantics/semantics_node.h @@ -46,17 +46,22 @@ enum class SemanticsAction : int32_t { kScrollToOffset = 1 << 23, }; -const int kVerticalScrollSemanticsActions = +constexpr int kVerticalScrollSemanticsActions = static_cast(SemanticsAction::kScrollUp) | static_cast(SemanticsAction::kScrollDown); -const int kHorizontalScrollSemanticsActions = +constexpr int kHorizontalScrollSemanticsActions = static_cast(SemanticsAction::kScrollLeft) | static_cast(SemanticsAction::kScrollRight); -const int kScrollableSemanticsActions = +constexpr int kScrollableSemanticsActions = kVerticalScrollSemanticsActions | kHorizontalScrollSemanticsActions; +/// The following actions are not user-initiated. +constexpr int kSystemActions = + static_cast(SemanticsAction::kDidGainAccessibilityFocus) | + static_cast(SemanticsAction::kDidLoseAccessibilityFocus); + /// C/C++ representation of `SemanticsRole` defined in /// `lib/ui/semantics.dart`. ///\warning This must match the `SemanticsRole` enum in diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index 39882196e8..2b6dfd9dde 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -781,6 +781,19 @@ CGRect ConvertRectToGlobal(SemanticsObject* reference, CGRect local_rect) { } } +- (BOOL)accessibilityRespondsToUserInteraction { + // Return true only if the node contains actions other than system actions. + if ((self.node.actions & ~flutter::kSystemActions) != 0) { + return true; + } + + if (!self.node.customAccessibilityActions.empty()) { + return true; + } + + return false; +} + @end @implementation FlutterSemanticsObject diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm index 4a553eb735..0f6ce68926 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm @@ -757,6 +757,162 @@ fml::RefPtr CreateNewThread(const std::string& name) { XCTAssertNil(rootNode.accessibilityValue); } +- (void)testSemanticObjectWithNoAccessibilityFlagNotMarkedAsResponsiveToUserInteraction { + flutter::MockDelegate mock_delegate; + auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest"); + flutter::TaskRunners runners(/*label=*/self.name.UTF8String, + /*platform=*/thread_task_runner, + /*raster=*/thread_task_runner, + /*ui=*/thread_task_runner, + /*io=*/thread_task_runner); + auto platform_view = std::make_unique( + /*delegate=*/mock_delegate, + /*rendering_api=*/mock_delegate.settings_.enable_impeller + ? flutter::IOSRenderingAPI::kMetal + : flutter::IOSRenderingAPI::kSoftware, + /*platform_views_controller=*/nil, + /*task_runners=*/runners, + /*worker_task_runner=*/nil, + /*is_gpu_disabled_sync_switch=*/std::make_shared()); + id engine = OCMClassMock([FlutterEngine class]); + id mockFlutterViewController = OCMClassMock([FlutterViewController class]); + FlutterView* flutterView = [[FlutterView alloc] initWithDelegate:engine + opaque:YES + enableWideGamut:NO]; + OCMStub([mockFlutterViewController view]).andReturn(flutterView); + auto ios_delegate = std::make_unique(); + __block auto bridge = + std::make_unique(/*view_controller=*/mockFlutterViewController, + /*platform_view=*/platform_view.get(), + /*platform_views_controller=*/nil, + /*ios_delegate=*/std::move(ios_delegate)); + + flutter::CustomAccessibilityActionUpdates actions; + flutter::SemanticsNodeUpdates nodes; + + flutter::SemanticsNode root_node; + root_node.id = kRootNodeId; + + nodes[root_node.id] = root_node; + bridge->UpdateSemantics(/*nodes=*/nodes, /*actions=*/actions); + + SemanticsObjectContainer* rootContainer = flutterView.accessibilityElements[0]; + FlutterSemanticsObject* rootNode = [rootContainer accessibilityElementAtIndex:0]; + + XCTAssertFalse(rootNode.accessibilityRespondsToUserInteraction); +} + +- (void)testSemanticObjectWithAccessibilityFlagsMarkedAsResponsiveToUserInteraction { + flutter::MockDelegate mock_delegate; + auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest"); + flutter::TaskRunners runners(/*label=*/self.name.UTF8String, + /*platform=*/thread_task_runner, + /*raster=*/thread_task_runner, + /*ui=*/thread_task_runner, + /*io=*/thread_task_runner); + auto platform_view = std::make_unique( + /*delegate=*/mock_delegate, + /*rendering_api=*/mock_delegate.settings_.enable_impeller + ? flutter::IOSRenderingAPI::kMetal + : flutter::IOSRenderingAPI::kSoftware, + /*platform_views_controller=*/nil, + /*task_runners=*/runners, + /*worker_task_runner=*/nil, + /*is_gpu_disabled_sync_switch=*/std::make_shared()); + id engine = OCMClassMock([FlutterEngine class]); + id mockFlutterViewController = OCMClassMock([FlutterViewController class]); + FlutterView* flutterView = [[FlutterView alloc] initWithDelegate:engine + opaque:YES + enableWideGamut:NO]; + OCMStub([mockFlutterViewController view]).andReturn(flutterView); + auto ios_delegate = std::make_unique(); + __block auto bridge = + std::make_unique(/*view_controller=*/mockFlutterViewController, + /*platform_view=*/platform_view.get(), + /*platform_views_controller=*/nil, + /*ios_delegate=*/std::move(ios_delegate)); + + flutter::CustomAccessibilityActionUpdates actions; + flutter::SemanticsNodeUpdates nodes; + + flutter::SemanticsNode root_node; + root_node.id = kRootNodeId; + root_node.actions = static_cast(flutter::SemanticsAction::kTap); + + nodes[root_node.id] = root_node; + bridge->UpdateSemantics(/*nodes=*/nodes, /*actions=*/actions); + + SemanticsObjectContainer* rootContainer = flutterView.accessibilityElements[0]; + FlutterSemanticsObject* rootNode = [rootContainer accessibilityElementAtIndex:0]; + + XCTAssertTrue(rootNode.accessibilityRespondsToUserInteraction); +} + +// Regression test for: +// https://github.com/flutter/flutter/issues/158477 +- (void)testLabeledParentAndChildNotInteractive { + flutter::MockDelegate mock_delegate; + auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest"); + flutter::TaskRunners runners(/*label=*/self.name.UTF8String, + /*platform=*/thread_task_runner, + /*raster=*/thread_task_runner, + /*ui=*/thread_task_runner, + /*io=*/thread_task_runner); + + FlutterPlatformViewsController* flutterPlatformViewsController = + [[FlutterPlatformViewsController alloc] init]; + auto platform_view = std::make_unique( + /*delegate=*/mock_delegate, + /*rendering_api=*/mock_delegate.settings_.enable_impeller + ? flutter::IOSRenderingAPI::kMetal + : flutter::IOSRenderingAPI::kSoftware, + /*platform_views_controller=*/flutterPlatformViewsController, + /*task_runners=*/runners, + /*worker_task_runner=*/nil, + /*is_gpu_disabled_sync_switch=*/std::make_shared()); + id engine = OCMClassMock([FlutterEngine class]); + id mockFlutterViewController = OCMClassMock([FlutterViewController class]); + FlutterView* flutterView = [[FlutterView alloc] initWithDelegate:engine + opaque:YES + enableWideGamut:NO]; + OCMStub([mockFlutterViewController view]).andReturn(flutterView); + + @autoreleasepool { + auto bridge = std::make_unique( + /*view_controller=*/mockFlutterViewController, + /*platform_view=*/platform_view.get(), + /*platform_views_controller=*/flutterPlatformViewsController); + + flutter::SemanticsNodeUpdates nodes; + + flutter::SemanticsNode parent; + parent.id = 0; + parent.rect = SkRect::MakeXYWH(0, 0, 100, 200); + parent.label = "parent_label"; + + flutter::SemanticsNode node; + node.id = 1; + node.rect = SkRect::MakeXYWH(0, 0, 100, 200); + node.label = "child_label"; + + parent.childrenInTraversalOrder.push_back(1); + parent.childrenInHitTestOrder.push_back(1); + nodes[0] = parent; + nodes[1] = node; + flutter::CustomAccessibilityActionUpdates actions; + bridge->UpdateSemantics(/*nodes=*/nodes, /*actions=*/actions); + + SemanticsObjectContainer* parentContainer = flutterView.accessibilityElements[0]; + FlutterSemanticsObject* parentNode = [parentContainer accessibilityElementAtIndex:0]; + FlutterSemanticsObject* childNode = [parentContainer accessibilityElementAtIndex:1]; + + XCTAssertTrue([parentNode.accessibilityLabel isEqualToString:@"parent_label"]); + XCTAssertTrue([childNode.accessibilityLabel isEqualToString:@"child_label"]); + XCTAssertFalse(parentNode.accessibilityRespondsToUserInteraction); + XCTAssertFalse(childNode.accessibilityRespondsToUserInteraction); + } +} + - (void)testLayoutChangeWithNonAccessibilityElement { flutter::MockDelegate mock_delegate; auto thread_task_runner = CreateNewThread("AccessibilityBridgeTest");