Skip to content

Commit

Permalink
Revert "[web] switch from .didGain/LoseAccessibilityFocus to .focus (f…
Browse files Browse the repository at this point in the history
…lutter#53134)"

This reverts commit a22270b.
  • Loading branch information
yjbanov committed Jun 11, 2024
1 parent 40cbeed commit b6f0ead
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 105 deletions.
24 changes: 0 additions & 24 deletions lib/web_ui/lib/src/engine/dom.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
17 changes: 13 additions & 4 deletions lib/web_ui/lib/src/engine/semantics/focusable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -132,6 +135,7 @@ class AccessibilityFocusManager {
semanticsNodeId: semanticsNodeId,
element: previousTarget.element,
domFocusListener: previousTarget.domFocusListener,
domBlurListener: previousTarget.domBlurListener,
);
return;
}
Expand All @@ -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.
Expand All @@ -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) {
Expand All @@ -177,7 +184,9 @@ class AccessibilityFocusManager {

EnginePlatformDispatcher.instance.invokeOnSemanticsAction(
target.semanticsNodeId,
ui.SemanticsAction.focus,
acquireFocus
? ui.SemanticsAction.didGainAccessibilityFocus
: ui.SemanticsAction.didLoseAccessibilityFocus,
null,
);
}
Expand Down Expand Up @@ -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
Expand Down
29 changes: 14 additions & 15 deletions lib/web_ui/lib/src/engine/semantics/text_field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}));
}

Expand Down Expand Up @@ -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.
Expand All @@ -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();
Expand Down
64 changes: 31 additions & 33 deletions lib/web_ui/test/engine/semantics/semantics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1776,7 +1776,7 @@ void _testIncrementables() {

pumpSemantics(isFocused: true);
expect(capturedActions, <CapturedAction>[
(0, ui.SemanticsAction.focus, null),
(0, ui.SemanticsAction.didGainAccessibilityFocus, null),
]);
capturedActions.clear();

Expand All @@ -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, <CapturedAction>[
(0, ui.SemanticsAction.didLoseAccessibilityFocus, null),
]);

semantics().semanticsEnabled = false;
});
Expand Down Expand Up @@ -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(
Expand All @@ -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),
Expand All @@ -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
Expand Down Expand Up @@ -2157,7 +2156,7 @@ void _testCheckables() {

pumpSemantics(isFocused: true);
expect(capturedActions, <CapturedAction>[
(0, ui.SemanticsAction.focus, null),
(0, ui.SemanticsAction.didGainAccessibilityFocus, null),
]);
capturedActions.clear();

Expand All @@ -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, <CapturedAction>[
(0, ui.SemanticsAction.didLoseAccessibilityFocus, null),
]);

semantics().semanticsEnabled = false;
});
Expand Down Expand Up @@ -2338,19 +2340,17 @@ void _testTappable() {

pumpSemantics(isFocused: true);
expect(capturedActions, <CapturedAction>[
(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, <CapturedAction>[
(0, ui.SemanticsAction.didLoseAccessibilityFocus, null),
]);

semantics().semanticsEnabled = false;
});
Expand Down Expand Up @@ -3180,7 +3180,7 @@ void _testDialog() {
expect(
capturedActions,
<CapturedAction>[
(2, ui.SemanticsAction.focus, null),
(2, ui.SemanticsAction.didGainAccessibilityFocus, null),
],
);

Expand Down Expand Up @@ -3242,7 +3242,7 @@ void _testDialog() {
expect(
capturedActions,
<CapturedAction>[
(3, ui.SemanticsAction.focus, null),
(3, ui.SemanticsAction.didGainAccessibilityFocus, null),
],
);

Expand Down Expand Up @@ -3392,7 +3392,7 @@ void _testFocusable() {
pumpSemantics(); // triggers post-update callbacks
expect(domDocument.activeElement, element);
expect(capturedActions, <CapturedAction>[
(1, ui.SemanticsAction.focus, null),
(1, ui.SemanticsAction.didGainAccessibilityFocus, null),
]);
capturedActions.clear();

Expand All @@ -3405,19 +3405,17 @@ 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, <CapturedAction>[
(1, ui.SemanticsAction.didLoseAccessibilityFocus, null),
]);
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),
(1, ui.SemanticsAction.didGainAccessibilityFocus, null),
]);
capturedActions.clear();

Expand Down
4 changes: 0 additions & 4 deletions lib/web_ui/test/engine/semantics/semantics_tester.dart
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class SemanticsTester {
bool? hasPaste,
bool? hasDidGainAccessibilityFocus,
bool? hasDidLoseAccessibilityFocus,
bool? hasFocus,
bool? hasCustomAction,
bool? hasDismiss,
bool? hasMoveCursorForwardByWord,
Expand Down Expand Up @@ -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;
}
Expand Down
Loading

0 comments on commit b6f0ead

Please sign in to comment.