From ce667ede75ce09f4c0268ee6094965640fc41675 Mon Sep 17 00:00:00 2001 From: chunhtai <47866232+chunhtai@users.noreply.github.com> Date: Tue, 18 Mar 2025 14:01:16 -0700 Subject: [PATCH] Reland role merge (#165330) the previous pr causes some issue with mobile platform. The change is in second commit ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../flutter/lib/src/semantics/semantics.dart | 20 ++ .../flutter/test/material/search_test.dart | 157 ++++++++++---- packages/flutter/test/widgets/basic_test.dart | 203 ++++++++++++++++++ 3 files changed, 334 insertions(+), 46 deletions(-) diff --git a/packages/flutter/lib/src/semantics/semantics.dart b/packages/flutter/lib/src/semantics/semantics.dart index 6ca090b9135..ff670d41707 100644 --- a/packages/flutter/lib/src/semantics/semantics.dart +++ b/packages/flutter/lib/src/semantics/semantics.dart @@ -5575,6 +5575,23 @@ class SemanticsConfiguration { bool _hasFlag(SemanticsFlag flag) => (_flags & flag.index) != 0; + bool get _hasExplicitRole { + if (_role != SemanticsRole.none) { + return true; + } + if (_hasFlag(SemanticsFlag.isTextField) || + // In non web platforms, the header is a trait. + (_hasFlag(SemanticsFlag.isHeader) && kIsWeb) || + _hasFlag(SemanticsFlag.isSlider) || + _hasFlag(SemanticsFlag.isLink) || + _hasFlag(SemanticsFlag.scopesRoute) || + _hasFlag(SemanticsFlag.isImage) || + _hasFlag(SemanticsFlag.isKeyboardKey)) { + return true; + } + return false; + } + // CONFIGURATION COMBINATION LOGIC /// Whether this configuration is compatible with the provided `other` @@ -5604,6 +5621,9 @@ class SemanticsConfiguration { if (_attributedValue.string.isNotEmpty && other._attributedValue.string.isNotEmpty) { return false; } + if (_hasExplicitRole && other._hasExplicitRole) { + return false; + } return true; } diff --git a/packages/flutter/test/material/search_test.dart b/packages/flutter/test/material/search_test.dart index aad47130128..8ee6da85bcf 100644 --- a/packages/flutter/test/material/search_test.dart +++ b/packages/flutter/test/material/search_test.dart @@ -651,6 +651,61 @@ void main() { final bool isCupertino = debugDefaultTargetPlatformOverride == TargetPlatform.iOS || debugDefaultTargetPlatformOverride == TargetPlatform.macOS; + final TestSemantics textField = + kIsWeb + ? TestSemantics( + flags: [ + SemanticsFlag.isHeader, + if (!isCupertino) SemanticsFlag.namesRoute, + ], + children: [ + TestSemantics( + id: 9, + flags: [ + SemanticsFlag.isTextField, + SemanticsFlag.hasEnabledState, + SemanticsFlag.isEnabled, + SemanticsFlag.isFocused, + ], + actions: [ + if (isDesktop) SemanticsAction.didGainAccessibilityFocus, + if (isDesktop) SemanticsAction.didLoseAccessibilityFocus, + SemanticsAction.tap, + SemanticsAction.focus, + SemanticsAction.setSelection, + SemanticsAction.setText, + SemanticsAction.paste, + ], + label: 'Search', + textDirection: TextDirection.ltr, + textSelection: const TextSelection(baseOffset: 0, extentOffset: 0), + ), + ], + ) + : TestSemantics( + id: 9, + flags: [ + SemanticsFlag.isTextField, + SemanticsFlag.hasEnabledState, + SemanticsFlag.isEnabled, + SemanticsFlag.isFocused, + SemanticsFlag.isHeader, + if (!isCupertino) SemanticsFlag.namesRoute, + ], + actions: [ + if (isDesktop) SemanticsAction.didGainAccessibilityFocus, + if (isDesktop) SemanticsAction.didLoseAccessibilityFocus, + SemanticsAction.tap, + SemanticsAction.focus, + SemanticsAction.setSelection, + SemanticsAction.setText, + SemanticsAction.paste, + ], + label: 'Search', + textDirection: TextDirection.ltr, + textSelection: const TextSelection(baseOffset: 0, extentOffset: 0), + ); + return TestSemantics.root( children: [ TestSemantics( @@ -688,29 +743,7 @@ void main() { tooltip: 'Back', textDirection: TextDirection.ltr, ), - TestSemantics( - id: 9, - flags: [ - SemanticsFlag.isTextField, - SemanticsFlag.hasEnabledState, - SemanticsFlag.isEnabled, - SemanticsFlag.isFocused, - SemanticsFlag.isHeader, - if (!isCupertino) SemanticsFlag.namesRoute, - ], - actions: [ - if (isDesktop) SemanticsAction.didGainAccessibilityFocus, - if (isDesktop) SemanticsAction.didLoseAccessibilityFocus, - SemanticsAction.tap, - SemanticsAction.focus, - SemanticsAction.setSelection, - SemanticsAction.setText, - SemanticsAction.paste, - ], - label: 'Search', - textDirection: TextDirection.ltr, - textSelection: const TextSelection(baseOffset: 0, extentOffset: 0), - ), + textField, TestSemantics( id: 10, label: 'Bottom', @@ -818,6 +851,60 @@ void main() { final bool isCupertino = debugDefaultTargetPlatformOverride == TargetPlatform.iOS || debugDefaultTargetPlatformOverride == TargetPlatform.macOS; + final TestSemantics textField = + kIsWeb + ? TestSemantics( + flags: [ + SemanticsFlag.isHeader, + if (!isCupertino) SemanticsFlag.namesRoute, + ], + children: [ + TestSemantics( + id: 11, + flags: [ + SemanticsFlag.isTextField, + SemanticsFlag.hasEnabledState, + SemanticsFlag.isEnabled, + SemanticsFlag.isFocused, + ], + actions: [ + if (isDesktop) SemanticsAction.didGainAccessibilityFocus, + if (isDesktop) SemanticsAction.didLoseAccessibilityFocus, + SemanticsAction.tap, + SemanticsAction.focus, + SemanticsAction.setSelection, + SemanticsAction.setText, + SemanticsAction.paste, + ], + label: 'Search', + textDirection: TextDirection.ltr, + textSelection: const TextSelection(baseOffset: 0, extentOffset: 0), + ), + ], + ) + : TestSemantics( + id: 11, + flags: [ + SemanticsFlag.isTextField, + SemanticsFlag.hasEnabledState, + SemanticsFlag.isEnabled, + SemanticsFlag.isFocused, + SemanticsFlag.isHeader, + if (!isCupertino) SemanticsFlag.namesRoute, + ], + actions: [ + if (isDesktop) SemanticsAction.didGainAccessibilityFocus, + if (isDesktop) SemanticsAction.didLoseAccessibilityFocus, + SemanticsAction.tap, + SemanticsAction.focus, + SemanticsAction.setSelection, + SemanticsAction.setText, + SemanticsAction.paste, + ], + label: 'Search', + textDirection: TextDirection.ltr, + textSelection: const TextSelection(baseOffset: 0, extentOffset: 0), + ); return TestSemantics.root( children: [ TestSemantics( @@ -852,29 +939,7 @@ void main() { tooltip: 'Back', textDirection: TextDirection.ltr, ), - TestSemantics( - id: 11, - flags: [ - SemanticsFlag.isTextField, - SemanticsFlag.hasEnabledState, - SemanticsFlag.isEnabled, - SemanticsFlag.isFocused, - SemanticsFlag.isHeader, - if (!isCupertino) SemanticsFlag.namesRoute, - ], - actions: [ - if (isDesktop) SemanticsAction.didGainAccessibilityFocus, - if (isDesktop) SemanticsAction.didLoseAccessibilityFocus, - SemanticsAction.tap, - SemanticsAction.focus, - SemanticsAction.setSelection, - SemanticsAction.setText, - SemanticsAction.paste, - ], - label: 'Search', - textDirection: TextDirection.ltr, - textSelection: const TextSelection(baseOffset: 0, extentOffset: 0), - ), + textField, TestSemantics(id: 14, label: 'Bottom', textDirection: TextDirection.ltr), ], ), diff --git a/packages/flutter/test/widgets/basic_test.dart b/packages/flutter/test/widgets/basic_test.dart index 5310051e57a..7af66de9485 100644 --- a/packages/flutter/test/widgets/basic_test.dart +++ b/packages/flutter/test/widgets/basic_test.dart @@ -11,6 +11,7 @@ import 'dart:math' as math; import 'dart:ui' as ui; import 'dart:ui'; +import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; @@ -380,6 +381,208 @@ void main() { expect(attributedHint.attributes[0].range, const TextRange(start: 1, end: 2)); }); + testWidgets('Semantics does not merge role', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics( + key: key2, + role: SemanticsRole.alertDialog, + child: const Placeholder(), + ), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.role, SemanticsRole.alertDialog); + }); + + testWidgets('Semantics does not merge role - text field', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics(key: key2, textField: true, child: const Placeholder()), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.isTextField), isTrue); + }); + + testWidgets('Semantics does not merge role - link', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics(key: key2, link: true, child: const Placeholder()), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.isLink), isTrue); + }); + + testWidgets('Semantics does not merge role - scopes route', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics( + key: key2, + scopesRoute: true, + explicitChildNodes: true, + child: const Placeholder(), + ), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.scopesRoute), isTrue); + }); + + testWidgets('Semantics does not merge role - header on web', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics(key: key2, header: true, child: const Placeholder()), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + if (kIsWeb) { + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.isHeader), isTrue); + } else { + expect(node1 == node2, isTrue); + } + }); + + testWidgets('Semantics does not merge role - image', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics(key: key2, image: true, child: const Placeholder()), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.isImage), isTrue); + }); + + testWidgets('Semantics does not merge role - slider', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics(key: key2, slider: true, child: const Placeholder()), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.isSlider), isTrue); + }); + + testWidgets('Semantics does not merge role - keyboard key', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics(key: key2, keyboardKey: true, child: const Placeholder()), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.isKeyboardKey), isTrue); + }); + + testWidgets('Semantics does not merge role - scopes route', (WidgetTester tester) async { + final UniqueKey key1 = UniqueKey(); + final UniqueKey key2 = UniqueKey(); + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: Semantics( + key: key1, + role: SemanticsRole.dialog, + child: Semantics(key: key2, slider: true, child: const Placeholder()), + ), + ), + ), + ); + final SemanticsNode node1 = tester.getSemantics(find.byKey(key1)); + final SemanticsNode node2 = tester.getSemantics(find.byKey(key2)); + expect(node1 != node2, isTrue); + expect(node1.role, SemanticsRole.dialog); + expect(node2.hasFlag(SemanticsFlag.isSlider), isTrue); + }); + testWidgets('Semantics can set controls visibility of nodes', (WidgetTester tester) async { final UniqueKey key = UniqueKey(); await tester.pumpWidget(