diff --git a/packages/flutter/lib/src/semantics/binding.dart b/packages/flutter/lib/src/semantics/binding.dart index 9636159817..5c4a03a790 100644 --- a/packages/flutter/lib/src/semantics/binding.dart +++ b/packages/flutter/lib/src/semantics/binding.dart @@ -75,6 +75,23 @@ mixin SemanticsBinding on BindingBase { _semanticsEnabled.removeListener(listener); } + final ObserverList> _semanticsActionListeners = + ObserverList>(); + + /// Adds a listener that is called for every [SemanticsActionEvent] received. + /// + /// The listeners are called before [performSemanticsAction] is invoked. + /// + /// To remove the listener, call [removeSemanticsActionListener]. + void addSemanticsActionListener(ValueSetter listener) { + _semanticsActionListeners.add(listener); + } + + /// Removes a listener previously added with [addSemanticsActionListener]. + void removeSemanticsActionListener(ValueSetter listener) { + _semanticsActionListeners.remove(listener); + } + /// The number of clients registered to listen for semantics. /// /// The number is increased whenever [ensureSemantics] is called and decreased @@ -125,6 +142,15 @@ mixin SemanticsBinding on BindingBase { arguments is ByteData ? action.copyWith(arguments: const StandardMessageCodec().decodeMessage(arguments)) : action; + // Listeners may get added/removed while the iteration is in progress. Since the list cannot + // be modified while iterating, we are creating a local copy for the iteration. + final List> localListeners = _semanticsActionListeners + .toList(growable: false); + for (final ValueSetter listener in localListeners) { + if (_semanticsActionListeners.contains(listener)) { + listener(decodedAction); + } + } performSemanticsAction(decodedAction); } diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart index adede810bd..f289a6ae43 100644 --- a/packages/flutter/lib/src/widgets/focus_manager.dart +++ b/packages/flutter/lib/src/widgets/focus_manager.dart @@ -13,6 +13,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/painting.dart'; import 'package:flutter/scheduler.dart'; +import 'package:flutter/semantics.dart'; import 'package:flutter/services.dart'; import 'binding.dart'; @@ -2064,9 +2065,9 @@ class _HighlightModeManager { } } - // If set, indicates if the last interaction detected was touch or not. If - // null, no interactions have occurred yet. - bool? _lastInteractionWasTouch; + // If null, no interactions have occurred yet and the default highlight mode for the current + // platform applies. + bool? _lastInteractionRequiresTraditionalHighlights; FocusHighlightMode get highlightMode => _highlightMode ?? _defaultModeForPlatform; FocusHighlightMode? _highlightMode; @@ -2122,6 +2123,7 @@ class _HighlightModeManager { // HardwareKeyboard. ServicesBinding.instance.keyEventManager.keyMessageHandler = handleKeyMessage; GestureBinding.instance.pointerRouter.addGlobalRoute(handlePointerEvent); + SemanticsBinding.instance.addSemanticsActionListener(handleSemanticsAction); } @mustCallSuper @@ -2132,6 +2134,7 @@ class _HighlightModeManager { if (ServicesBinding.instance.keyEventManager.keyMessageHandler == handleKeyMessage) { GestureBinding.instance.pointerRouter.removeGlobalRoute(handlePointerEvent); ServicesBinding.instance.keyEventManager.keyMessageHandler = null; + SemanticsBinding.instance.removeSemanticsActionListener(handleSemanticsAction); } _listeners = HashedObserverList>(); } @@ -2175,29 +2178,28 @@ class _HighlightModeManager { } void handlePointerEvent(PointerEvent event) { - final FocusHighlightMode expectedMode; switch (event.kind) { case PointerDeviceKind.touch: case PointerDeviceKind.stylus: case PointerDeviceKind.invertedStylus: - _lastInteractionWasTouch = true; - expectedMode = FocusHighlightMode.touch; + if (_lastInteractionRequiresTraditionalHighlights != true) { + _lastInteractionRequiresTraditionalHighlights = true; + updateMode(); + } case PointerDeviceKind.mouse: case PointerDeviceKind.trackpad: case PointerDeviceKind.unknown: - _lastInteractionWasTouch = false; - expectedMode = FocusHighlightMode.traditional; - } - if (expectedMode != highlightMode) { - updateMode(); } } bool handleKeyMessage(KeyMessage message) { - // Update highlightMode first, since things responding to the keys might - // look at the highlight mode, and it should be accurate. - _lastInteractionWasTouch = false; - updateMode(); + // ignore: use_if_null_to_convert_nulls_to_bools + if (_lastInteractionRequiresTraditionalHighlights != false) { + // Update highlightMode first, since things responding to the keys might + // look at the highlight mode, and it should be accurate. + _lastInteractionRequiresTraditionalHighlights = false; + updateMode(); + } assert(_focusDebug(() => 'Received key event $message')); if (FocusManager.instance.primaryFocus == null) { @@ -2290,20 +2292,29 @@ class _HighlightModeManager { return handled; } + void handleSemanticsAction(SemanticsActionEvent semanticsActionEvent) { + if (kIsWeb && + semanticsActionEvent.type == SemanticsAction.focus && + _lastInteractionRequiresTraditionalHighlights != true) { + _lastInteractionRequiresTraditionalHighlights = true; + updateMode(); + } + } + // Update function to be called whenever the state relating to highlightMode // changes. void updateMode() { final FocusHighlightMode newMode; switch (strategy) { case FocusHighlightStrategy.automatic: - if (_lastInteractionWasTouch == null) { + if (_lastInteractionRequiresTraditionalHighlights == null) { // If we don't have any information about the last interaction yet, // then just rely on the default value for the platform, which will be // determined based on the target platform if _highlightMode is not // set. return; } - if (_lastInteractionWasTouch!) { + if (_lastInteractionRequiresTraditionalHighlights!) { newMode = FocusHighlightMode.touch; } else { newMode = FocusHighlightMode.traditional; diff --git a/packages/flutter/test/material/icon_button_test.dart b/packages/flutter/test/material/icon_button_test.dart index 92cd2fb5ca..735e52cbb3 100644 --- a/packages/flutter/test/material/icon_button_test.dart +++ b/packages/flutter/test/material/icon_button_test.dart @@ -9,6 +9,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; + import '../widgets/feedback_tester.dart'; import '../widgets/semantics_tester.dart'; @@ -3317,6 +3318,62 @@ void main() { await tester.pump(); expect(onLongPressed, false); }); + + testWidgets('does not draw focus color when focused by semantics on the web', ( + WidgetTester tester, + ) async { + // Regression test for https://github.com/flutter/flutter/issues/158527. + + final FocusNode focusNode = FocusNode(); + addTearDown(focusNode.dispose); + + const Color focusColor = Colors.orange; + + await tester.pumpWidget( + MaterialApp( + home: Center( + child: IconButton( + focusColor: focusColor, + focusNode: focusNode, + icon: const Icon(Icons.headphones), + onPressed: () {}, + ), + ), + ), + ); + + // Make sure we are in "traditional mode" where the button could potentially draw focus highlights. + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + await tester.pumpAndSettle(); + expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.traditional)); + + expect(focusNode.hasFocus, isFalse); + + // Focus on it with semantics. + tester.platformDispatcher.onSemanticsActionEvent!( + SemanticsActionEvent( + type: SemanticsAction.focus, + viewId: tester.view.viewId, + nodeId: tester.semantics.find(find.byIcon(Icons.headphones)).id, + ), + ); + await tester.pumpAndSettle(); + + // Make sure no focus highlight was drawn. + final RenderObject inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) { + return object.runtimeType.toString() == '_RenderInkFeatures'; + }); + expect(focusNode.hasFocus, isTrue); + expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.touch)); + expect(inkFeatures, isNot(paints..rect(color: focusColor))); + + // Check that focus highlight is drawn in traditional mode. + await tester.sendKeyEvent(LogicalKeyboardKey.enter); + await tester.pumpAndSettle(); + expect(focusNode.hasFocus, isTrue); + expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.traditional)); + expect(inkFeatures, paints..rect(color: focusColor)); + }, skip: !isBrowser); // [intended] tests web-specific behavior. } Widget buildAllVariants({ diff --git a/packages/flutter/test/material/toggle_buttons_test.dart b/packages/flutter/test/material/toggle_buttons_test.dart index ed6fe72aff..a29cbbb285 100644 --- a/packages/flutter/test/material/toggle_buttons_test.dart +++ b/packages/flutter/test/material/toggle_buttons_test.dart @@ -12,6 +12,7 @@ import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter_test/flutter_test.dart'; + import '../widgets/semantics_tester.dart'; const double _defaultBorderWidth = 1.0; @@ -692,6 +693,7 @@ void main() { await tester.pumpAndSettle(); // focusColor + FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; focusNode.requestFocus(); await tester.pumpAndSettle(); inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) { @@ -748,6 +750,7 @@ void main() { await tester.pumpAndSettle(); // focusColor + FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; focusNode.requestFocus(); await tester.pumpAndSettle(); inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) { @@ -811,6 +814,7 @@ void main() { await hoverGesture.moveTo(Offset.zero); // focusColor + FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; focusNode.requestFocus(); await tester.pumpAndSettle(); inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) { diff --git a/packages/flutter/test/material/toggle_buttons_theme_test.dart b/packages/flutter/test/material/toggle_buttons_theme_test.dart index edf4dffba6..74c8828b55 100644 --- a/packages/flutter/test/material/toggle_buttons_theme_test.dart +++ b/packages/flutter/test/material/toggle_buttons_theme_test.dart @@ -482,6 +482,7 @@ void main() { await hoverGesture.moveTo(Offset.zero); // focusColor + FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; focusNode.requestFocus(); await tester.pumpAndSettle(); inkFeatures = tester.allRenderObjects.firstWhere((RenderObject object) { diff --git a/packages/flutter/test/widgets/focus_manager_test.dart b/packages/flutter/test/widgets/focus_manager_test.dart index 04dd246d50..77ad78f7a3 100644 --- a/packages/flutter/test/widgets/focus_manager_test.dart +++ b/packages/flutter/test/widgets/focus_manager_test.dart @@ -1519,19 +1519,19 @@ void main() { kind: PointerDeviceKind.mouse, ); await gesture.up(); - expect(callCount, equals(3)); - expect(lastMode, FocusHighlightMode.traditional); - expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.traditional)); + expect(callCount, equals(2)); + expect(lastMode, FocusHighlightMode.touch); + expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.touch)); await tester.tap(find.byType(Container), warnIfMissed: false); - expect(callCount, equals(4)); + expect(callCount, equals(2)); expect(lastMode, FocusHighlightMode.touch); expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.touch)); FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; - expect(callCount, equals(5)); + expect(callCount, equals(3)); expect(lastMode, FocusHighlightMode.traditional); expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.traditional)); FocusManager.instance.highlightStrategy = FocusHighlightStrategy.alwaysTouch; - expect(callCount, equals(6)); + expect(callCount, equals(4)); expect(lastMode, FocusHighlightMode.touch); expect(FocusManager.instance.highlightMode, equals(FocusHighlightMode.touch)); });