From b6f0eadc4fc4460d71f6f5886486ac7a55ed1552 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Tue, 11 Jun 2024 13:25:05 -0700 Subject: [PATCH] Revert "[web] switch from .didGain/LoseAccessibilityFocus to .focus (#53134)" This reverts commit a22270bc8419cb12d2560482476af3d61cc7978d. --- lib/web_ui/lib/src/engine/dom.dart | 24 ------- .../lib/src/engine/semantics/focusable.dart | 17 +++-- .../lib/src/engine/semantics/text_field.dart | 29 ++++----- .../test/engine/semantics/semantics_test.dart | 64 +++++++++---------- .../engine/semantics/semantics_tester.dart | 4 -- .../engine/semantics/text_field_test.dart | 33 +++------- 6 files changed, 66 insertions(+), 105 deletions(-) diff --git a/lib/web_ui/lib/src/engine/dom.dart b/lib/web_ui/lib/src/engine/dom.dart index 9dc23ba24e4d1..b11e9e97a7458 100644 --- a/lib/web_ui/lib/src/engine/dom.dart +++ b/lib/web_ui/lib/src/engine/dom.dart @@ -2737,30 +2737,6 @@ 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/lib/web_ui/lib/src/engine/semantics/focusable.dart b/lib/web_ui/lib/src/engine/semantics/focusable.dart index 331e1cd50c061..35fff64a50158 100644 --- a/lib/web_ui/lib/src/engine/semantics/focusable.dart +++ b/lib/web_ui/lib/src/engine/semantics/focusable.dart @@ -81,6 +81,9 @@ 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. @@ -132,6 +135,7 @@ class AccessibilityFocusManager { semanticsNodeId: semanticsNodeId, element: previousTarget.element, domFocusListener: previousTarget.domFocusListener, + domBlurListener: previousTarget.domBlurListener, ); return; } @@ -144,12 +148,14 @@ class AccessibilityFocusManager { final _FocusTarget newTarget = ( semanticsNodeId: semanticsNodeId, element: element, - domFocusListener: createDomEventListener((_) => _didReceiveDomFocus()), + domFocusListener: createDomEventListener((_) => _setFocusFromDom(true)), + domBlurListener: createDomEventListener((_) => _setFocusFromDom(false)), ); _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. @@ -164,9 +170,10 @@ class AccessibilityFocusManager { } target.element.removeEventListener('focus', target.domFocusListener); + target.element.removeEventListener('blur', target.domBlurListener); } - void _didReceiveDomFocus() { + void _setFocusFromDom(bool acquireFocus) { final _FocusTarget? target = _target; if (target == null) { @@ -177,7 +184,9 @@ class AccessibilityFocusManager { EnginePlatformDispatcher.instance.invokeOnSemanticsAction( target.semanticsNodeId, - ui.SemanticsAction.focus, + acquireFocus + ? ui.SemanticsAction.didGainAccessibilityFocus + : ui.SemanticsAction.didLoseAccessibilityFocus, null, ); } @@ -220,7 +229,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 - // SemanticsAction.focus action. Screen readers on the web do not do + // didGainAccessibilityFocus 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/lib/web_ui/lib/src/engine/semantics/text_field.dart b/lib/web_ui/lib/src/engine/semantics/text_field.dart index f43c2d6eb27ee..bb79ea1df52d9 100644 --- a/lib/web_ui/lib/src/engine/semantics/text_field.dart +++ b/lib/web_ui/lib/src/engine/semantics/text_field.dart @@ -257,7 +257,6 @@ class TextField extends PrimaryRoleManager { editableElement = semanticsObject.hasFlag(ui.SemanticsFlag.isMultiline) ? createDomHTMLTextAreaElement() : createDomHTMLInputElement(); - _updateEnabledState(); // On iOS, even though the semantic text field is transparent, the cursor // and text highlighting are still visible. The cursor and text selection @@ -311,7 +310,16 @@ class TextField extends PrimaryRoleManager { } EnginePlatformDispatcher.instance.invokeOnSemanticsAction( - semanticsObject.id, ui.SemanticsAction.focus, null); + semanticsObject.id, ui.SemanticsAction.didGainAccessibilityFocus, null); + })); + activeEditableElement.addEventListener('blur', + createDomEventListener((DomEvent event) { + if (EngineSemantics.instance.gestureMode != GestureMode.browserGestures) { + return; + } + + EnginePlatformDispatcher.instance.invokeOnSemanticsAction( + semanticsObject.id, ui.SemanticsAction.didLoseAccessibilityFocus, null); })); } @@ -425,19 +433,20 @@ class TextField extends PrimaryRoleManager { // and wait for a tap event before invoking the iOS workaround and creating // the editable element. if (editableElement != null) { - _updateEnabledState(); activeEditableElement.style ..width = '${semanticsObject.rect!.width}px' ..height = '${semanticsObject.rect!.height}px'; if (semanticsObject.hasFocus) { - if (domDocument.activeElement != activeEditableElement && semanticsObject.isEnabled) { + if (domDocument.activeElement != + activeEditableElement) { semanticsObject.owner.addOneTimePostUpdateCallback(() { activeEditableElement.focus(); }); } SemanticsTextEditingStrategy._instance?.activate(this); - } else if (domDocument.activeElement == activeEditableElement) { + } else if (domDocument.activeElement == + activeEditableElement) { if (!isIosSafari) { SemanticsTextEditingStrategy._instance?.deactivate(this); // Only apply text, because this node is not focused. @@ -457,16 +466,6 @@ class TextField extends PrimaryRoleManager { } } - void _updateEnabledState() { - final DomElement? element = editableElement; - - if (element == null) { - return; - } - - (element as DomElementWithDisabledProperty).disabled = !semanticsObject.isEnabled; - } - @override void dispose() { super.dispose(); diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index 2288d8b0bb961..117ebc55cb60a 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -1776,7 +1776,7 @@ void _testIncrementables() { pumpSemantics(isFocused: true); expect(capturedActions, [ - (0, ui.SemanticsAction.focus, null), + (0, ui.SemanticsAction.didGainAccessibilityFocus, null), ]); capturedActions.clear(); @@ -1787,12 +1787,10 @@ void _testIncrementables() { 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); + expect(capturedActions, [ + (0, ui.SemanticsAction.didLoseAccessibilityFocus, null), + ]); semantics().semanticsEnabled = false; }); @@ -1823,14 +1821,15 @@ void _testTextField() { final SemanticsObject node = owner().debugSemanticsTree![0]!; - final TextField textFieldRole = node.primaryRole! as TextField; - final DomHTMLInputElement inputElement = textFieldRole.activeEditableElement as DomHTMLInputElement; // TODO(yjbanov): this used to attempt to test that value="hello" but the // test was a false positive. We should revise this test and // make sure it tests the right things: // https://github.com/flutter/flutter/issues/147200 - expect(inputElement.value, ''); + expect( + (node.element as DomHTMLInputElement).value, + isNull, + ); expect(node.primaryRole?.role, PrimaryRole.textField); expect( @@ -1853,8 +1852,8 @@ void _testTextField() { final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); updateNode( builder, - actions: 0 | ui.SemanticsAction.focus.index, - flags: 0 | ui.SemanticsFlag.isTextField.index | ui.SemanticsFlag.isEnabled.index, + actions: 0 | ui.SemanticsAction.didGainAccessibilityFocus.index, + flags: 0 | ui.SemanticsFlag.isTextField.index, value: 'hello', transform: Matrix4.identity().toFloat64(), rect: const ui.Rect.fromLTRB(0, 0, 100, 50), @@ -1871,7 +1870,7 @@ void _testTextField() { expect(owner().semanticsHost.ownerDocument?.activeElement, textField); expect(await logger.idLog.first, 0); - expect(await logger.actionLog.first, ui.SemanticsAction.focus); + expect(await logger.actionLog.first, ui.SemanticsAction.didGainAccessibilityFocus); semantics().semanticsEnabled = false; }, // TODO(yjbanov): https://github.com/flutter/flutter/issues/46638 @@ -2157,7 +2156,7 @@ void _testCheckables() { pumpSemantics(isFocused: true); expect(capturedActions, [ - (0, ui.SemanticsAction.focus, null), + (0, ui.SemanticsAction.didGainAccessibilityFocus, null), ]); capturedActions.clear(); @@ -2167,12 +2166,15 @@ void _testCheckables() { 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. + // If the element is blurred by the browser, then we do want to notify the + // framework. This is because screen reader can be focused on something + // other than what the framework is focused on, and notifying the framework + // about the loss of focus on a node is information that the framework did + // not have before. element.blur(); - expect(capturedActions, isEmpty); + expect(capturedActions, [ + (0, ui.SemanticsAction.didLoseAccessibilityFocus, null), + ]); semantics().semanticsEnabled = false; }); @@ -2338,19 +2340,17 @@ void _testTappable() { pumpSemantics(isFocused: true); expect(capturedActions, [ - (0, ui.SemanticsAction.focus, null), + (0, ui.SemanticsAction.didGainAccessibilityFocus, null), ]); 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); + expect(capturedActions, [ + (0, ui.SemanticsAction.didLoseAccessibilityFocus, null), + ]); semantics().semanticsEnabled = false; }); @@ -3180,7 +3180,7 @@ void _testDialog() { expect( capturedActions, [ - (2, ui.SemanticsAction.focus, null), + (2, ui.SemanticsAction.didGainAccessibilityFocus, null), ], ); @@ -3242,7 +3242,7 @@ void _testDialog() { expect( capturedActions, [ - (3, ui.SemanticsAction.focus, null), + (3, ui.SemanticsAction.didGainAccessibilityFocus, null), ], ); @@ -3392,7 +3392,7 @@ void _testFocusable() { pumpSemantics(); // triggers post-update callbacks expect(domDocument.activeElement, element); expect(capturedActions, [ - (1, ui.SemanticsAction.focus, null), + (1, ui.SemanticsAction.didGainAccessibilityFocus, null), ]); capturedActions.clear(); @@ -3405,11 +3405,9 @@ void _testFocusable() { // Browser blurs the element element.blur(); expect(domDocument.activeElement, isNot(element)); - // 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. - expect(capturedActions, isEmpty); + expect(capturedActions, [ + (1, ui.SemanticsAction.didLoseAccessibilityFocus, null), + ]); capturedActions.clear(); // Request focus again @@ -3417,7 +3415,7 @@ void _testFocusable() { pumpSemantics(); // triggers post-update callbacks expect(domDocument.activeElement, element); expect(capturedActions, [ - (1, ui.SemanticsAction.focus, null), + (1, ui.SemanticsAction.didGainAccessibilityFocus, null), ]); capturedActions.clear(); diff --git a/lib/web_ui/test/engine/semantics/semantics_tester.dart b/lib/web_ui/test/engine/semantics/semantics_tester.dart index e14a2c6a18132..f9a626c486cf7 100644 --- a/lib/web_ui/test/engine/semantics/semantics_tester.dart +++ b/lib/web_ui/test/engine/semantics/semantics_tester.dart @@ -75,7 +75,6 @@ class SemanticsTester { bool? hasPaste, bool? hasDidGainAccessibilityFocus, bool? hasDidLoseAccessibilityFocus, - bool? hasFocus, bool? hasCustomAction, bool? hasDismiss, bool? hasMoveCursorForwardByWord, @@ -243,9 +242,6 @@ class SemanticsTester { if (hasDidLoseAccessibilityFocus ?? false) { actions |= ui.SemanticsAction.didLoseAccessibilityFocus.index; } - if (hasFocus ?? false) { - actions |= ui.SemanticsAction.focus.index; - } if (hasCustomAction ?? false) { actions |= ui.SemanticsAction.customAction.index; } diff --git a/lib/web_ui/test/engine/semantics/text_field_test.dart b/lib/web_ui/test/engine/semantics/text_field_test.dart index f1dc9203f1647..aee5e1b331a52 100644 --- a/lib/web_ui/test/engine/semantics/text_field_test.dart +++ b/lib/web_ui/test/engine/semantics/text_field_test.dart @@ -102,26 +102,15 @@ void testMain() { // make sure it tests the right things: // https://github.com/flutter/flutter/issues/147200 final SemanticsObject node = owner().debugSemanticsTree![0]!; - final TextField textFieldRole = node.primaryRole! as TextField; - final DomHTMLInputElement inputElement = textFieldRole.activeEditableElement as DomHTMLInputElement; - expect(inputElement.tagName.toLowerCase(), 'input'); - expect(inputElement.value, ''); - expect(inputElement.disabled, isFalse); - }); - - test('renders a disabled text field', () { - createTextFieldSemantics(isEnabled: false, value: 'hello'); - expectSemanticsTree(owner(), ''''''); - final SemanticsObject node = owner().debugSemanticsTree![0]!; - final TextField textFieldRole = node.primaryRole! as TextField; - final DomHTMLInputElement inputElement = textFieldRole.activeEditableElement as DomHTMLInputElement; - expect(inputElement.tagName.toLowerCase(), 'input'); - expect(inputElement.disabled, isTrue); + expect( + (node.element as DomHTMLInputElement).value, + isNull, + ); }); // TODO(yjbanov): this test will need to be adjusted for Safari when we add // Safari testing. - test('sends a SemanticsAction.focus action when browser requests focus', () async { + test('sends a didGainAccessibilityFocus/didLoseAccessibilityFocus action when browser requests focus/blur', () async { final SemanticsActionLogger logger = SemanticsActionLogger(); createTextFieldSemantics(value: 'hello'); @@ -134,11 +123,13 @@ void testMain() { expect(owner().semanticsHost.ownerDocument?.activeElement, textField); expect(await logger.idLog.first, 0); - expect(await logger.actionLog.first, ui.SemanticsAction.focus); + expect(await logger.actionLog.first, ui.SemanticsAction.didGainAccessibilityFocus); textField.blur(); expect(owner().semanticsHost.ownerDocument?.activeElement, isNot(textField)); + expect(await logger.idLog.first, 0); + expect(await logger.actionLog.first, ui.SemanticsAction.didLoseAccessibilityFocus); }, // TODO(yjbanov): https://github.com/flutter/flutter/issues/46638 // TODO(yjbanov): https://github.com/flutter/flutter/issues/50590 skip: ui_web.browser.browserEngine != ui_web.BrowserEngine.blink); @@ -436,7 +427,6 @@ void testMain() { children: [ builder.updateNode( id: 1, - isEnabled: true, isTextField: true, value: 'Hello', isFocused: focusFieldId == 1, @@ -444,7 +434,6 @@ void testMain() { ), builder.updateNode( id: 2, - isEnabled: true, isTextField: true, value: 'World', isFocused: focusFieldId == 2, @@ -895,7 +884,6 @@ void testMain() { SemanticsObject createTextFieldSemantics({ required String value, String label = '', - bool isEnabled = true, bool isFocused = false, bool isMultiline = false, ui.Rect rect = const ui.Rect.fromLTRB(0, 0, 100, 50), @@ -905,7 +893,6 @@ SemanticsObject createTextFieldSemantics({ final SemanticsTester tester = SemanticsTester(owner()); tester.updateNode( id: 0, - isEnabled: isEnabled, label: label, value: value, isTextField: true, @@ -986,7 +973,6 @@ Map createTwoFieldSemanticsForIos(SemanticsTester builder, children: [ builder.updateNode( id: 1, - isEnabled: true, isTextField: true, value: 'Hello', label: 'Hello', @@ -995,7 +981,6 @@ Map createTwoFieldSemanticsForIos(SemanticsTester builder, ), builder.updateNode( id: 2, - isEnabled: true, isTextField: true, value: 'World', label: 'World', @@ -1016,7 +1001,6 @@ Map createTwoFieldSemanticsForIos(SemanticsTester builder, children: [ builder.updateNode( id: 1, - isEnabled: true, isTextField: true, value: 'Hello', label: 'Hello', @@ -1025,7 +1009,6 @@ Map createTwoFieldSemanticsForIos(SemanticsTester builder, ), builder.updateNode( id: 2, - isEnabled: true, isTextField: true, value: 'World', label: 'World',