do not echo synthetic focus requests

Author: yjbanov

lib/web_ui/lib/src/engine/semantics/focusable.dart

@@ -48,6 +48,7 @@ class Focusable extends RoleManager {
   /// this role manager did not take the focus. The return value can be used to
   /// decide whether to stop searching for a node that should take focus.
   bool focusAsRouteDefault() {
+    _focusManager._lastEvent = AccessibilityFocusManagerEvent.requestedFocus;
     owner.element.focus();
     return true;
   }
@@ -81,8 +82,26 @@ typedef _FocusTarget = ({
 
   /// The listener for the "focus" DOM event.
   DomEventListener domFocusListener,
+
+  /// The listener for the "blur" DOM event.
+  DomEventListener domBlurListener,
 });
 
+enum AccessibilityFocusManagerEvent {
+  /// No event has happend for the target element.
+  nothing,
+
+  /// The engine requested focus on the DOM element, possibly because the
+  /// framework requested it.
+  requestedFocus,
+
+  /// Received the DOM "focus" event.
+  receivedDomFocus,
+
+  /// Received the DOM "blur" event.
+  receivedDomBlur,
+}
+
 /// Implements accessibility focus management for arbitrary elements.
 ///
 /// Unlike [Focusable], which implements focus features on [SemanticsObject]s
@@ -99,6 +118,8 @@ class AccessibilityFocusManager {
 
   _FocusTarget? _target;
 
+  AccessibilityFocusManagerEvent _lastEvent = AccessibilityFocusManagerEvent.nothing;
+
   // The last focus value set by this focus manager, used to prevent requesting
   // focus on the same element repeatedly. Requesting focus on DOM elements is
   // not an idempotent operation. If the element is already focused and focus is
@@ -132,6 +153,7 @@ class AccessibilityFocusManager {
         semanticsNodeId: semanticsNodeId,
         element: previousTarget.element,
         domFocusListener: previousTarget.domFocusListener,
+        domBlurListener: previousTarget.domBlurListener,
       );
       return;
     }
@@ -145,11 +167,14 @@ class AccessibilityFocusManager {
       semanticsNodeId: semanticsNodeId,
       element: element,
       domFocusListener: createDomEventListener((_) => _didReceiveDomFocus()),
+      domBlurListener: createDomEventListener((_) => _didReceiveDomBlur()),
     );
     _target = newTarget;
+    _lastEvent = AccessibilityFocusManagerEvent.nothing;
 
     element.tabIndex = 0;
     element.addEventListener('focus', newTarget.domFocusListener);
+    element.addEventListener('blur', newTarget.domBlurListener);
   }
 
   /// Stops managing the focus of the current element, if any.
@@ -164,6 +189,7 @@ class AccessibilityFocusManager {
     }
 
     target.element.removeEventListener('focus', target.domFocusListener);
+    target.element.removeEventListener('blur', target.domBlurListener);
   }
 
   void _didReceiveDomFocus() {
@@ -175,11 +201,23 @@ class AccessibilityFocusManager {
       return;
     }
 
-    EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
-      target.semanticsNodeId,
-      ui.SemanticsAction.focus,
-      null,
-    );
+    // Do not notify the framework if DOM focus was acquired as a result of
+    // requesting it programmatically. Only notify the framework if the DOM
+    // focus was initiated by the browser, e.g. as a result of the screen reader
+    // shifting focus.
+    if (_lastEvent != AccessibilityFocusManagerEvent.requestedFocus) {
+      EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
+        target.semanticsNodeId,
+        ui.SemanticsAction.focus,
+        null,
+      );
+    }
+
+    _lastEvent = AccessibilityFocusManagerEvent.receivedDomFocus;
+  }
+
+  void _didReceiveDomBlur() {
+    _lastEvent = AccessibilityFocusManagerEvent.receivedDomBlur;
   }
 
   /// Requests focus or blur on the DOM element.
@@ -240,6 +278,7 @@ class AccessibilityFocusManager {
         return;
       }
 
+      _lastEvent = AccessibilityFocusManagerEvent.requestedFocus;
       target.element.focus();
     });
   }

lib/web_ui/test/engine/semantics/semantics_test.dart

@@ -1844,9 +1844,22 @@ void _testIncrementables() {
     expect(capturedActions, isEmpty);
 
     pumpSemantics(isFocused: true);
-    expect(capturedActions, <CapturedAction>[
-      (0, ui.SemanticsAction.focus, null),
-    ]);
+    expect(
+      reason: 'Framework requested focus. No need to circle the event back to the framework.',
+      capturedActions,
+      isEmpty,
+    );
+    capturedActions.clear();
+
+    element.blur();
+    element.focus();
+    expect(
+      reason: 'Browser-initiated focus even should be communicated to the framework.',
+      capturedActions,
+      <CapturedAction>[
+        (0, ui.SemanticsAction.focus, null),
+      ],
+    );
     capturedActions.clear();
 
     pumpSemantics(isFocused: false);
@@ -2189,24 +2202,30 @@ void _testCheckables() {
     expect(capturedActions, isEmpty);
 
     pumpSemantics(isFocused: true);
-    expect(capturedActions, <CapturedAction>[
-      (0, ui.SemanticsAction.focus, null),
-    ]);
+    expect(
+      reason: 'Framework requested focus. No need to circle the event back to the framework.',
+      capturedActions,
+      isEmpty,
+    );
     capturedActions.clear();
 
-    // The framework removes focus from the widget (i.e. "blurs" it). Since the
-    // blurring is initiated by the framework, there's no need to send any
-    // notifications back to the framework about it.
-    pumpSemantics(isFocused: false);
-    expect(capturedActions, isEmpty);
-
     // The web doesn't send didLoseAccessibilityFocus as on the web,
     // accessibility focus is not observable, only input focus is. As of this
     // writing, there is no SemanticsAction.unfocus action, so the test simply
     // asserts that no actions are being sent as a result of blur.
     element.blur();
     expect(capturedActions, isEmpty);
 
+    element.focus();
+    expect(
+      reason: 'Browser-initiated focus even should be communicated to the framework.',
+      capturedActions,
+      <CapturedAction>[
+        (0, ui.SemanticsAction.focus, null),
+      ],
+    );
+    capturedActions.clear();
+
     semantics().semanticsEnabled = false;
   });
 }
@@ -2370,21 +2389,34 @@ void _testTappable() {
     expect(capturedActions, isEmpty);
 
     pumpSemantics(isFocused: true);
-    expect(capturedActions, <CapturedAction>[
-      (0, ui.SemanticsAction.focus, null),
-    ]);
+    expect(
+      reason: 'Framework requested focus. No need to circle the event back to the framework.',
+      capturedActions,
+      isEmpty,
+    );
+    expect(domDocument.activeElement, element);
     capturedActions.clear();
 
-    pumpSemantics(isFocused: false);
-    expect(capturedActions, isEmpty);
-
     // The web doesn't send didLoseAccessibilityFocus as on the web,
     // accessibility focus is not observable, only input focus is. As of this
     // writing, there is no SemanticsAction.unfocus action, so the test simply
     // asserts that no actions are being sent as a result of blur.
     element.blur();
     expect(capturedActions, isEmpty);
 
+    element.focus();
+    expect(
+      reason: 'Browser-initiated focus even should be communicated to the framework.',
+      capturedActions,
+      <CapturedAction>[
+        (0, ui.SemanticsAction.focus, null),
+      ],
+    );
+    capturedActions.clear();
+
+    pumpSemantics(isFocused: false);
+    expect(capturedActions, isEmpty);
+
     semantics().semanticsEnabled = false;
   });
 
@@ -3210,12 +3242,8 @@ void _testDialog() {
     );
     tester.apply();
 
-    expect(
-      capturedActions,
-      <CapturedAction>[
-        (2, ui.SemanticsAction.focus, null),
-      ],
-    );
+    // Auto-focus does not notify the framework about the focused widget.
+    expect(capturedActions, isEmpty);
 
     semantics().semanticsEnabled = false;
   });
@@ -3272,12 +3300,8 @@ void _testDialog() {
     );
     tester.apply();
 
-    expect(
-      capturedActions,
-      <CapturedAction>[
-        (3, ui.SemanticsAction.focus, null),
-      ],
-    );
+    // Auto-focus does not notify the framework about the focused widget.
+    expect(capturedActions, isEmpty);
 
     semantics().semanticsEnabled = false;
   });
@@ -3424,10 +3448,7 @@ void _testFocusable() {
     manager.changeFocus(true);
     pumpSemantics(); // triggers post-update callbacks
     expect(domDocument.activeElement, element);
-    expect(capturedActions, <CapturedAction>[
-      (1, ui.SemanticsAction.focus, null),
-    ]);
-    capturedActions.clear();
+    expect(capturedActions, isEmpty);
 
     // Give up focus
     manager.changeFocus(false);
@@ -3443,16 +3464,12 @@ void _testFocusable() {
     // writing, there is no SemanticsAction.unfocus action, so the test simply
     // asserts that no actions are being sent as a result of blur.
     expect(capturedActions, isEmpty);
-    capturedActions.clear();
 
     // Request focus again
     manager.changeFocus(true);
     pumpSemantics(); // triggers post-update callbacks
     expect(domDocument.activeElement, element);
-    expect(capturedActions, <CapturedAction>[
-      (1, ui.SemanticsAction.focus, null),
-    ]);
-    capturedActions.clear();
+    expect(capturedActions, isEmpty);
 
     // Double-request focus
     manager.changeFocus(true);
@@ -3463,6 +3480,16 @@ void _testFocusable() {
       capturedActions, isEmpty);
     capturedActions.clear();
 
+    // Blur and emulate browser requesting focus
+    element.blur();
+    expect(domDocument.activeElement, isNot(element));
+    element.focus();
+    expect(domDocument.activeElement, element);
+    expect(capturedActions, <CapturedAction>[
+      (1, ui.SemanticsAction.focus, null),
+    ]);
+    capturedActions.clear();
+
     // Stop managing
     manager.stopManaging();
     pumpSemantics(); // triggers post-update callbacks