Reland "Use semantics label for backbutton and closebutton for Androi…

…d" (flutter#116676) * Reland "Use semantics label for backbutton and closebutton for Android" This reverts commit cc256c3. * Makes the semantics slider test more robust
engine-flutter-autoroll · Dec 7, 2022 · 521028c · 521028c
1 parent 8b80552
commit 521028c
Show file tree

Hide file tree

Showing 4 changed files with 164 additions and 29 deletions.
diff --git a/packages/flutter/lib/src/material/back_button.dart b/packages/flutter/lib/src/material/back_button.dart
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:flutter/foundation.dart';
 import 'package:flutter/widgets.dart';
 
 import 'debug.dart';
@@ -27,22 +28,39 @@ class BackButtonIcon extends StatelessWidget {
   /// the current platform (as obtained from the [Theme]).
   const BackButtonIcon({ super.key });
 
-  /// Returns the appropriate "back" icon for the given `platform`.
-  static IconData _getIconData(TargetPlatform platform) {
-    switch (platform) {
+  @override
+  Widget build(BuildContext context) {
+    final String? semanticsLabel;
+    final IconData data;
+    switch (Theme.of(context).platform) {
       case TargetPlatform.android:
       case TargetPlatform.fuchsia:
       case TargetPlatform.linux:
       case TargetPlatform.windows:
-        return Icons.arrow_back;
+        data = Icons.arrow_back;
+        break;
       case TargetPlatform.iOS:
       case TargetPlatform.macOS:
-        return Icons.arrow_back_ios;
+        data = Icons.arrow_back_ios;
+        break;
+    }
+    // This can't use the platform from Theme because it is the Android OS that
+    // expects the duplicated tooltip and label.
+    switch (defaultTargetPlatform) {
+      case TargetPlatform.android:
+        semanticsLabel = MaterialLocalizations.of(context).backButtonTooltip;
+        break;
+      case TargetPlatform.fuchsia:
+      case TargetPlatform.linux:
+      case TargetPlatform.windows:
+      case TargetPlatform.iOS:
+      case TargetPlatform.macOS:
+        semanticsLabel = null;
+        break;
     }
-  }
 
-  @override
-  Widget build(BuildContext context) => Icon(_getIconData(Theme.of(context).platform));
+    return Icon(data, semanticLabel: semanticsLabel);
+  }
 }
 
 /// A Material Design back button.
@@ -149,8 +167,23 @@ class CloseButton extends StatelessWidget {
   @override
   Widget build(BuildContext context) {
     assert(debugCheckHasMaterialLocalizations(context));
+    final String? semanticsLabel;
+    // This can't use the platform from Theme because it is the Android OS that
+    // expects the duplicated tooltip and label.
+    switch (defaultTargetPlatform) {
+      case TargetPlatform.android:
+        semanticsLabel = MaterialLocalizations.of(context).closeButtonTooltip;
+        break;
+      case TargetPlatform.fuchsia:
+      case TargetPlatform.linux:
+      case TargetPlatform.windows:
+      case TargetPlatform.iOS:
+      case TargetPlatform.macOS:
+        semanticsLabel = null;
+        break;
+    }
     return IconButton(
-      icon: const Icon(Icons.close),
+      icon: Icon(Icons.close, semanticLabel: semanticsLabel),
       color: color,
       tooltip: MaterialLocalizations.of(context).closeButtonTooltip,
       onPressed: () {

diff --git a/packages/flutter/lib/src/widgets/semantics_debugger.dart b/packages/flutter/lib/src/widgets/semantics_debugger.dart
@@ -288,10 +288,14 @@ class _SemanticsDebuggerPainter extends CustomPainter {
 
     assert(data.attributedLabel != null);
     final String message;
+    // Android will avoid pronouncing duplicating tooltip and label.
+    // Therefore, having two identical strings is the same as having a single
+    // string.
+    final bool shouldIgnoreDuplicatedLabel = defaultTargetPlatform == TargetPlatform.android && data.attributedLabel.string == data.tooltip;
     final String tooltipAndLabel = <String>[
       if (data.tooltip.isNotEmpty)
         data.tooltip,
-      if (data.attributedLabel.string.isNotEmpty)
+      if (data.attributedLabel.string.isNotEmpty && !shouldIgnoreDuplicatedLabel)
         data.attributedLabel.string,
     ].join('\n');
     if (tooltipAndLabel.isEmpty) {

diff --git a/packages/flutter/test/material/back_button_test.dart b/packages/flutter/test/material/back_button_test.dart
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 
@@ -152,17 +153,73 @@ void main() {
     tester.state<NavigatorState>(find.byType(Navigator)).pushNamed('/next');
 
     await tester.pumpAndSettle();
-
+    final String? expectedLabel;
+    switch(defaultTargetPlatform) {
+      case TargetPlatform.android:
+        expectedLabel = 'Back';
+        break;
+      case TargetPlatform.fuchsia:
+      case TargetPlatform.iOS:
+      case TargetPlatform.linux:
+      case TargetPlatform.macOS:
+      case TargetPlatform.windows:
+        expectedLabel = null;
+    }
     expect(tester.getSemantics(find.byType(BackButton)), matchesSemantics(
       tooltip: 'Back',
+      label: expectedLabel,
       isButton: true,
       hasEnabledState: true,
       isEnabled: true,
       hasTapAction: true,
       isFocusable: true,
     ));
     handle.dispose();
-  });
+  }, variant: TargetPlatformVariant.all());
+
+  testWidgets('CloseButton semantics', (WidgetTester tester) async {
+    final SemanticsHandle handle = tester.ensureSemantics();
+    await tester.pumpWidget(
+      MaterialApp(
+        home: const Material(child: Text('Home')),
+        routes: <String, WidgetBuilder>{
+          '/next': (BuildContext context) {
+            return const Material(
+              child: Center(
+                child: CloseButton(),
+              ),
+            );
+          },
+        },
+      ),
+    );
+
+    tester.state<NavigatorState>(find.byType(Navigator)).pushNamed('/next');
+
+    await tester.pumpAndSettle();
+    final String? expectedLabel;
+    switch(defaultTargetPlatform) {
+      case TargetPlatform.android:
+        expectedLabel = 'Close';
+        break;
+      case TargetPlatform.fuchsia:
+      case TargetPlatform.iOS:
+      case TargetPlatform.linux:
+      case TargetPlatform.macOS:
+      case TargetPlatform.windows:
+        expectedLabel = null;
+    }
+    expect(tester.getSemantics(find.byType(CloseButton)), matchesSemantics(
+      tooltip: 'Close',
+      label: expectedLabel,
+      isButton: true,
+      hasEnabledState: true,
+      isEnabled: true,
+      hasTapAction: true,
+      isFocusable: true,
+    ));
+    handle.dispose();
+  }, variant: TargetPlatformVariant.all());
 
   testWidgets('CloseButton color', (WidgetTester tester) async {
     await tester.pumpWidget(

diff --git a/packages/flutter/test/widgets/semantics_debugger_test.dart b/packages/flutter/test/widgets/semantics_debugger_test.dart
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 
@@ -293,26 +294,28 @@ void main() {
     double value = 0.75;
 
     await tester.pumpWidget(
-      Directionality(
-        textDirection: TextDirection.ltr,
-        child: SemanticsDebugger(
-          child: Directionality(
-            textDirection: TextDirection.ltr,
-            child: MediaQuery(
-              data: MediaQueryData.fromWindow(WidgetsBinding.instance.window),
-              child: Material(
-                child: Center(
-                  child: Slider(
-                    value: value,
-                    onChanged: (double newValue) {
-                      value = newValue;
-                    },
+      MaterialApp(
+        home: Directionality(
+          textDirection: TextDirection.ltr,
+          child: SemanticsDebugger(
+            child: Directionality(
+              textDirection: TextDirection.ltr,
+              child: MediaQuery(
+                data: MediaQueryData.fromWindow(WidgetsBinding.instance.window),
+                child: Material(
+                  child: Center(
+                    child: Slider(
+                      value: value,
+                      onChanged: (double newValue) {
+                        value = newValue;
+                      },
+                    ),
                   ),
                 ),
               ),
             ),
           ),
-        ),
+        )
       ),
     );
 
@@ -322,8 +325,19 @@ void main() {
     // interpreted as a gesture by the semantics debugger and sent to the widget
     // as a semantic action that always moves by 10% of the complete track.
     await tester.fling(find.byType(Slider), const Offset(-100.0, 0.0), 2000.0, warnIfMissed: false); // hitting the debugger
-    expect(value, equals(0.70));
-  });
+    switch(defaultTargetPlatform) {
+      case TargetPlatform.iOS:
+      case TargetPlatform.macOS:
+        expect(value, equals(0.65));
+        break;
+      case TargetPlatform.linux:
+      case TargetPlatform.windows:
+      case TargetPlatform.android:
+      case TargetPlatform.fuchsia:
+        expect(value, equals(0.70));
+        break;
+    }
+  }, variant: TargetPlatformVariant.all());
 
   testWidgets('SemanticsDebugger checkbox', (WidgetTester tester) async {
     final Key keyTop = UniqueKey();
@@ -438,6 +452,33 @@ void main() {
     );
   });
 
+  testWidgets('SemanticsDebugger ignores duplicated label and tooltip for Android', (WidgetTester tester) async {
+    final Key child = UniqueKey();
+    final Key debugger = UniqueKey();
+    final bool isPlatformAndroid = defaultTargetPlatform == TargetPlatform.android;
+    await tester.pumpWidget(
+      Directionality(
+        textDirection: TextDirection.ltr,
+        child: SemanticsDebugger(
+          key: debugger,
+          child: Material(
+            child: Semantics(
+              container: true,
+              key: child,
+              label: 'text',
+              tooltip: 'text',
+            ),
+          ),
+        ),
+      ),
+    );
+
+    expect(
+      _getMessageShownInSemanticsDebugger(widgetKey: child, debuggerKey: debugger, tester: tester),
+      isPlatformAndroid ? 'text' : 'text\ntext',
+    );
+  }, variant: TargetPlatformVariant.all());
+
   testWidgets('SemanticsDebugger textfield', (WidgetTester tester) async {
     final UniqueKey textField = UniqueKey();
     final UniqueKey debugger = UniqueKey();