From c5a8a826efe6d304ef26f36bee0d5e062cf9206e Mon Sep 17 00:00:00 2001 From: Yegor Date: Fri, 28 Jun 2024 14:35:10 -0700 Subject: [PATCH] [web] switch from .didGain/LoseAccessibilityFocus to .focus (flutter/engine#53360) This is a repeat of https://github.com/flutter/engine/pull/53134, which was merged prematurely. > [!WARNING] > Only land this after: > * https://github.com/flutter/flutter/pull/149840 lands in the framework. > * You have personally manually tested the change together with the latest framework on all browsers. ## Original PR description Stop using `SemanticsAction.didGain/LoseAccessibilityFocus` on the web, start using `SemanticsAction.focus`. This is because on the web, a11y focus is not observable, only input focus is. Sending `SemanticsAction.focus` will guarantee that the framework move focus to the respective widget. There currently is no "unfocus" signal, because it seems to be already covered: either another widget gains focus, or an HTML DOM element outside the Flutter view does, both of which have their respective signals already. More details in the discussion in the issue https://github.com/flutter/flutter/issues/83809. Fixes https://github.com/flutter/flutter/issues/83809 Fixes https://github.com/flutter/flutter/issues/148285 Fixes https://github.com/flutter/flutter/issues/143337 --- engine/src/flutter/lib/ui/semantics.dart | 3 + .../lib/web_ui/lib/src/engine/dom.dart | 24 + .../lib/src/engine/semantics/focusable.dart | 17 +- .../lib/src/engine/semantics/semantics.dart | 2 - .../lib/src/engine/semantics/text_field.dart | 256 ++---- .../src/engine/text_editing/text_editing.dart | 24 +- .../test/engine/semantics/semantics_test.dart | 94 +-- .../engine/semantics/semantics_tester.dart | 4 + .../engine/semantics/text_field_test.dart | 731 ++++-------------- 9 files changed, 257 insertions(+), 898 deletions(-) diff --git a/engine/src/flutter/lib/ui/semantics.dart b/engine/src/flutter/lib/ui/semantics.dart index 57bc1fa307..481c9c3cbc 100644 --- a/engine/src/flutter/lib/ui/semantics.dart +++ b/engine/src/flutter/lib/ui/semantics.dart @@ -220,6 +220,9 @@ class SemanticsAction { /// must immediately become editable, opening a virtual keyboard, if needed. /// Buttons must respond to tap/click events from the keyboard. /// + /// Widget reaction to this action must be idempotent. It is possible to + /// receive this action more than once, or when the widget is already focused. + /// /// Focus behavior is specific to the platform and to the assistive technology /// used. Typically on desktop operating systems, such as Windows, macOS, and /// Linux, moving accessibility focus will also move the input focus. On diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart index 622cf3f2a2..42f6cfa1a2 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart @@ -2748,6 +2748,30 @@ DomCompositionEvent createDomCompositionEvent(String type, } } +/// This is a pseudo-type for DOM elements that have the boolean `disabled` +/// property. +/// +/// This type cannot be part of the actual type hierarchy because each DOM type +/// defines its `disabled` property ad hoc, without inheriting it from a common +/// type, e.g. [DomHTMLInputElement] and [DomHTMLTextAreaElement]. +/// +/// To use, simply cast any element known to have the `disabled` property to +/// this type using `as DomElementWithDisabledProperty`, then read and write +/// this property as normal. +@JS() +@staticInterop +class DomElementWithDisabledProperty extends DomHTMLElement {} + +extension DomElementWithDisabledPropertyExtension on DomElementWithDisabledProperty { + @JS('disabled') + external JSBoolean? get _disabled; + bool? get disabled => _disabled?.toDart; + + @JS('disabled') + external set _disabled(JSBoolean? value); + set disabled(bool? value) => _disabled = value?.toJS; +} + @JS() @staticInterop class DomHTMLInputElement extends DomHTMLElement {} diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart index 35fff64a50..331e1cd50c 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart @@ -81,9 +81,6 @@ typedef _FocusTarget = ({ /// The listener for the "focus" DOM event. DomEventListener domFocusListener, - - /// The listener for the "blur" DOM event. - DomEventListener domBlurListener, }); /// Implements accessibility focus management for arbitrary elements. @@ -135,7 +132,6 @@ class AccessibilityFocusManager { semanticsNodeId: semanticsNodeId, element: previousTarget.element, domFocusListener: previousTarget.domFocusListener, - domBlurListener: previousTarget.domBlurListener, ); return; } @@ -148,14 +144,12 @@ class AccessibilityFocusManager { final _FocusTarget newTarget = ( semanticsNodeId: semanticsNodeId, element: element, - domFocusListener: createDomEventListener((_) => _setFocusFromDom(true)), - domBlurListener: createDomEventListener((_) => _setFocusFromDom(false)), + domFocusListener: createDomEventListener((_) => _didReceiveDomFocus()), ); _target = newTarget; element.tabIndex = 0; element.addEventListener('focus', newTarget.domFocusListener); - element.addEventListener('blur', newTarget.domBlurListener); } /// Stops managing the focus of the current element, if any. @@ -170,10 +164,9 @@ class AccessibilityFocusManager { } target.element.removeEventListener('focus', target.domFocusListener); - target.element.removeEventListener('blur', target.domBlurListener); } - void _setFocusFromDom(bool acquireFocus) { + void _didReceiveDomFocus() { final _FocusTarget? target = _target; if (target == null) { @@ -184,9 +177,7 @@ class AccessibilityFocusManager { EnginePlatformDispatcher.instance.invokeOnSemanticsAction( target.semanticsNodeId, - acquireFocus - ? ui.SemanticsAction.didGainAccessibilityFocus - : ui.SemanticsAction.didLoseAccessibilityFocus, + ui.SemanticsAction.focus, null, ); } @@ -229,7 +220,7 @@ class AccessibilityFocusManager { // a dialog, and nothing else in the dialog is focused. The Flutter // framework expects that the screen reader will focus on the first (in // traversal order) focusable element inside the dialog and send a - // didGainAccessibilityFocus action. Screen readers on the web do not do + // SemanticsAction.focus action. Screen readers on the web do not do // that, and so the web engine has to implement this behavior directly. So // the dialog will look for a focusable element and request focus on it, // but now there may be a race between this method unsetting the focus and diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart index c48851d983..0918ef49c3 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -2218,8 +2218,6 @@ class EngineSemantics { 'mousemove', 'mouseleave', 'mouseup', - 'keyup', - 'keydown', ]; if (pointerEventTypes.contains(event.type)) { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart index bb79ea1df5..3618306d37 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart @@ -2,11 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; import 'package:ui/ui.dart' as ui; -import 'package:ui/ui_web/src/ui_web.dart' as ui_web; -import '../browser_detection.dart' show isIosSafari; import '../dom.dart'; import '../platform_dispatcher.dart'; import '../text_editing/text_editing.dart'; @@ -123,7 +120,10 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { // Android). // Otherwise, the keyboard stays on screen even when the user navigates to // a different screen (e.g. by hitting the "back" button). - domElement?.blur(); + // Keep this consistent with how DefaultTextEditingStrategy does it. As of + // right now, the only difference is that semantic text fields do not + // participate in form autofill. + DefaultTextEditingStrategy.scheduleFocusFlutterView(activeDomElement, activeDomElementView); domElement = null; activeTextField = null; _queuedStyle = null; @@ -162,7 +162,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { if (hasAutofillGroup) { placeForm(); } - activeDomElement.focus(); + activeDomElement.focus(preventScroll: true); } @override @@ -207,69 +207,40 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { /// [EngineSemanticsOwner.gestureMode]. However, in Chrome on Android it ignores /// browser gestures when in pointer mode. In Safari on iOS pointer events are /// used to detect text box invocation. This is because Safari issues touch -/// events even when Voiceover is enabled. +/// events even when VoiceOver is enabled. class TextField extends PrimaryRoleManager { TextField(SemanticsObject semanticsObject) : super.blank(PrimaryRole.textField, semanticsObject) { - _setupDomElement(); + _initializeEditableElement(); } - /// The element used for editing, e.g. ``, `