Skip to content

Commit c2bb3ac

Browse files
chunhtaichunhtai
authored
Prevent explicit roles from merging (#164732)
<!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> If merging explicit roles, some of the information may be dropped. ## 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]. <!-- Links --> [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
1 parent 02f3ddf commit c2bb3ac

File tree

3 files changed

+264
-32
lines changed

3 files changed

+264
-32
lines changed

‎packages/flutter/lib/src/semantics/semantics.dart‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5539,6 +5539,22 @@ class SemanticsConfiguration {
55395539

55405540
bool _hasFlag(SemanticsFlag flag) => (_flags & flag.index) != 0;
55415541

5542+
bool get _hasExplicitRole {
5543+
if (_role != SemanticsRole.none) {
5544+
return true;
5545+
}
5546+
if (_hasFlag(SemanticsFlag.isTextField) ||
5547+
_hasFlag(SemanticsFlag.isHeader) ||
5548+
_hasFlag(SemanticsFlag.isSlider) ||
5549+
_hasFlag(SemanticsFlag.isLink) ||
5550+
_hasFlag(SemanticsFlag.scopesRoute) ||
5551+
_hasFlag(SemanticsFlag.isImage) ||
5552+
_hasFlag(SemanticsFlag.isKeyboardKey)) {
5553+
return true;
5554+
}
5555+
return false;
5556+
}
5557+
55425558
// CONFIGURATION COMBINATION LOGIC
55435559

55445560
/// Whether this configuration is compatible with the provided `other`
@@ -5568,6 +5584,9 @@ class SemanticsConfiguration {
55685584
if (_attributedValue.string.isNotEmpty && other._attributedValue.string.isNotEmpty) {
55695585
return false;
55705586
}
5587+
if (_hasExplicitRole && other._hasExplicitRole) {
5588+
return false;
5589+
}
55715590
return true;
55725591
}
55735592

‎packages/flutter/test/material/search_test.dart‎

Lines changed: 47 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -689,27 +689,36 @@ void main() {
689689
textDirection: TextDirection.ltr,
690690
),
691691
TestSemantics(
692-
id: 9,
693692
flags: <SemanticsFlag>[
694-
SemanticsFlag.isTextField,
695-
SemanticsFlag.hasEnabledState,
696-
SemanticsFlag.isEnabled,
697-
SemanticsFlag.isFocused,
698693
SemanticsFlag.isHeader,
699694
if (!isCupertino) SemanticsFlag.namesRoute,
700695
],
701-
actions: <SemanticsAction>[
702-
if (isDesktop) SemanticsAction.didGainAccessibilityFocus,
703-
if (isDesktop) SemanticsAction.didLoseAccessibilityFocus,
704-
SemanticsAction.tap,
705-
SemanticsAction.focus,
706-
SemanticsAction.setSelection,
707-
SemanticsAction.setText,
708-
SemanticsAction.paste,
696+
children: <TestSemantics>[
697+
TestSemantics(
698+
id: 9,
699+
flags: <SemanticsFlag>[
700+
SemanticsFlag.isTextField,
701+
SemanticsFlag.hasEnabledState,
702+
SemanticsFlag.isEnabled,
703+
SemanticsFlag.isFocused,
704+
],
705+
actions: <SemanticsAction>[
706+
if (isDesktop) SemanticsAction.didGainAccessibilityFocus,
707+
if (isDesktop) SemanticsAction.didLoseAccessibilityFocus,
708+
SemanticsAction.tap,
709+
SemanticsAction.focus,
710+
SemanticsAction.setSelection,
711+
SemanticsAction.setText,
712+
SemanticsAction.paste,
713+
],
714+
label: 'Search',
715+
textDirection: TextDirection.ltr,
716+
textSelection: const TextSelection(
717+
baseOffset: 0,
718+
extentOffset: 0,
719+
),
720+
),
709721
],
710-
label: 'Search',
711-
textDirection: TextDirection.ltr,
712-
textSelection: const TextSelection(baseOffset: 0, extentOffset: 0),
713722
),
714723
TestSemantics(
715724
id: 10,
@@ -853,27 +862,33 @@ void main() {
853862
textDirection: TextDirection.ltr,
854863
),
855864
TestSemantics(
856-
id: 11,
857865
flags: <SemanticsFlag>[
858-
SemanticsFlag.isTextField,
859-
SemanticsFlag.hasEnabledState,
860-
SemanticsFlag.isEnabled,
861-
SemanticsFlag.isFocused,
862866
SemanticsFlag.isHeader,
863867
if (!isCupertino) SemanticsFlag.namesRoute,
864868
],
865-
actions: <SemanticsAction>[
866-
if (isDesktop) SemanticsAction.didGainAccessibilityFocus,
867-
if (isDesktop) SemanticsAction.didLoseAccessibilityFocus,
868-
SemanticsAction.tap,
869-
SemanticsAction.focus,
870-
SemanticsAction.setSelection,
871-
SemanticsAction.setText,
872-
SemanticsAction.paste,
869+
children: <TestSemantics>[
870+
TestSemantics(
871+
id: 11,
872+
flags: <SemanticsFlag>[
873+
SemanticsFlag.isTextField,
874+
SemanticsFlag.hasEnabledState,
875+
SemanticsFlag.isEnabled,
876+
SemanticsFlag.isFocused,
877+
],
878+
actions: <SemanticsAction>[
879+
if (isDesktop) SemanticsAction.didGainAccessibilityFocus,
880+
if (isDesktop) SemanticsAction.didLoseAccessibilityFocus,
881+
SemanticsAction.tap,
882+
SemanticsAction.focus,
883+
SemanticsAction.setSelection,
884+
SemanticsAction.setText,
885+
SemanticsAction.paste,
886+
],
887+
label: 'Search',
888+
textDirection: TextDirection.ltr,
889+
textSelection: const TextSelection(baseOffset: 0, extentOffset: 0),
890+
),
873891
],
874-
label: 'Search',
875-
textDirection: TextDirection.ltr,
876-
textSelection: const TextSelection(baseOffset: 0, extentOffset: 0),
877892
),
878893
TestSemantics(id: 14, label: 'Bottom', textDirection: TextDirection.ltr),
879894
],

‎packages/flutter/test/widgets/basic_test.dart‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,204 @@ void main() {
380380
expect(attributedHint.attributes[0].range, const TextRange(start: 1, end: 2));
381381
});
382382

383+
testWidgets('Semantics does not merge role', (WidgetTester tester) async {
384+
final UniqueKey key1 = UniqueKey();
385+
final UniqueKey key2 = UniqueKey();
386+
await tester.pumpWidget(
387+
MaterialApp(
388+
home: Scaffold(
389+
body: Semantics(
390+
key: key1,
391+
role: SemanticsRole.dialog,
392+
child: Semantics(
393+
key: key2,
394+
role: SemanticsRole.alertDialog,
395+
child: const Placeholder(),
396+
),
397+
),
398+
),
399+
),
400+
);
401+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
402+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
403+
expect(node1 != node2, isTrue);
404+
expect(node1.role, SemanticsRole.dialog);
405+
expect(node2.role, SemanticsRole.alertDialog);
406+
});
407+
408+
testWidgets('Semantics does not merge role - text field', (WidgetTester tester) async {
409+
final UniqueKey key1 = UniqueKey();
410+
final UniqueKey key2 = UniqueKey();
411+
await tester.pumpWidget(
412+
MaterialApp(
413+
home: Scaffold(
414+
body: Semantics(
415+
key: key1,
416+
role: SemanticsRole.dialog,
417+
child: Semantics(key: key2, textField: true, child: const Placeholder()),
418+
),
419+
),
420+
),
421+
);
422+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
423+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
424+
expect(node1 != node2, isTrue);
425+
expect(node1.role, SemanticsRole.dialog);
426+
expect(node2.hasFlag(SemanticsFlag.isTextField), isTrue);
427+
});
428+
429+
testWidgets('Semantics does not merge role - link', (WidgetTester tester) async {
430+
final UniqueKey key1 = UniqueKey();
431+
final UniqueKey key2 = UniqueKey();
432+
await tester.pumpWidget(
433+
MaterialApp(
434+
home: Scaffold(
435+
body: Semantics(
436+
key: key1,
437+
role: SemanticsRole.dialog,
438+
child: Semantics(key: key2, link: true, child: const Placeholder()),
439+
),
440+
),
441+
),
442+
);
443+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
444+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
445+
expect(node1 != node2, isTrue);
446+
expect(node1.role, SemanticsRole.dialog);
447+
expect(node2.hasFlag(SemanticsFlag.isLink), isTrue);
448+
});
449+
450+
testWidgets('Semantics does not merge role - scopes route', (WidgetTester tester) async {
451+
final UniqueKey key1 = UniqueKey();
452+
final UniqueKey key2 = UniqueKey();
453+
await tester.pumpWidget(
454+
MaterialApp(
455+
home: Scaffold(
456+
body: Semantics(
457+
key: key1,
458+
role: SemanticsRole.dialog,
459+
child: Semantics(
460+
key: key2,
461+
scopesRoute: true,
462+
explicitChildNodes: true,
463+
child: const Placeholder(),
464+
),
465+
),
466+
),
467+
),
468+
);
469+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
470+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
471+
expect(node1 != node2, isTrue);
472+
expect(node1.role, SemanticsRole.dialog);
473+
expect(node2.hasFlag(SemanticsFlag.scopesRoute), isTrue);
474+
});
475+
476+
testWidgets('Semantics does not merge role - header', (WidgetTester tester) async {
477+
final UniqueKey key1 = UniqueKey();
478+
final UniqueKey key2 = UniqueKey();
479+
await tester.pumpWidget(
480+
MaterialApp(
481+
home: Scaffold(
482+
body: Semantics(
483+
key: key1,
484+
role: SemanticsRole.dialog,
485+
child: Semantics(key: key2, header: true, child: const Placeholder()),
486+
),
487+
),
488+
),
489+
);
490+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
491+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
492+
expect(node1 != node2, isTrue);
493+
expect(node1.role, SemanticsRole.dialog);
494+
expect(node2.hasFlag(SemanticsFlag.isHeader), isTrue);
495+
});
496+
497+
testWidgets('Semantics does not merge role - image', (WidgetTester tester) async {
498+
final UniqueKey key1 = UniqueKey();
499+
final UniqueKey key2 = UniqueKey();
500+
await tester.pumpWidget(
501+
MaterialApp(
502+
home: Scaffold(
503+
body: Semantics(
504+
key: key1,
505+
role: SemanticsRole.dialog,
506+
child: Semantics(key: key2, image: true, child: const Placeholder()),
507+
),
508+
),
509+
),
510+
);
511+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
512+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
513+
expect(node1 != node2, isTrue);
514+
expect(node1.role, SemanticsRole.dialog);
515+
expect(node2.hasFlag(SemanticsFlag.isImage), isTrue);
516+
});
517+
518+
testWidgets('Semantics does not merge role - slider', (WidgetTester tester) async {
519+
final UniqueKey key1 = UniqueKey();
520+
final UniqueKey key2 = UniqueKey();
521+
await tester.pumpWidget(
522+
MaterialApp(
523+
home: Scaffold(
524+
body: Semantics(
525+
key: key1,
526+
role: SemanticsRole.dialog,
527+
child: Semantics(key: key2, slider: true, child: const Placeholder()),
528+
),
529+
),
530+
),
531+
);
532+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
533+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
534+
expect(node1 != node2, isTrue);
535+
expect(node1.role, SemanticsRole.dialog);
536+
expect(node2.hasFlag(SemanticsFlag.isSlider), isTrue);
537+
});
538+
539+
testWidgets('Semantics does not merge role - keyboard key', (WidgetTester tester) async {
540+
final UniqueKey key1 = UniqueKey();
541+
final UniqueKey key2 = UniqueKey();
542+
await tester.pumpWidget(
543+
MaterialApp(
544+
home: Scaffold(
545+
body: Semantics(
546+
key: key1,
547+
role: SemanticsRole.dialog,
548+
child: Semantics(key: key2, keyboardKey: true, child: const Placeholder()),
549+
),
550+
),
551+
),
552+
);
553+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
554+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
555+
expect(node1 != node2, isTrue);
556+
expect(node1.role, SemanticsRole.dialog);
557+
expect(node2.hasFlag(SemanticsFlag.isKeyboardKey), isTrue);
558+
});
559+
560+
testWidgets('Semantics does not merge role - scopes route', (WidgetTester tester) async {
561+
final UniqueKey key1 = UniqueKey();
562+
final UniqueKey key2 = UniqueKey();
563+
await tester.pumpWidget(
564+
MaterialApp(
565+
home: Scaffold(
566+
body: Semantics(
567+
key: key1,
568+
role: SemanticsRole.dialog,
569+
child: Semantics(key: key2, slider: true, child: const Placeholder()),
570+
),
571+
),
572+
),
573+
);
574+
final SemanticsNode node1 = tester.getSemantics(find.byKey(key1));
575+
final SemanticsNode node2 = tester.getSemantics(find.byKey(key2));
576+
expect(node1 != node2, isTrue);
577+
expect(node1.role, SemanticsRole.dialog);
578+
expect(node2.hasFlag(SemanticsFlag.isSlider), isTrue);
579+
});
580+
383581
testWidgets('Semantics can set controls visibility of nodes', (WidgetTester tester) async {
384582
final UniqueKey key = UniqueKey();
385583
await tester.pumpWidget(

0 commit comments

Comments
 (0)