Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] switch to SemanticsAction.focus (attempt 3) #53689

Merged
auto-submit merged 2 commits into from
Jul 15, 2024
Merged

[web] switch to SemanticsAction.focus (attempt 3) #53689

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e830845
[web] switch to SemanticsAction.focus
yjbanov Jul 2, 2024
a887f2b
do not echo synthetic focus requests
yjbanov Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions lib/web_ui/lib/src/engine/semantics/focusable.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -132,6 +153,7 @@ class AccessibilityFocusManager {
semanticsNodeId: semanticsNodeId,
element: previousTarget.element,
domFocusListener: previousTarget.domFocusListener,
domBlurListener: previousTarget.domBlurListener,
);
return;
}
Expand All @@ -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.
Expand All @@ -164,6 +189,7 @@ class AccessibilityFocusManager {
}

target.element.removeEventListener('focus', target.domFocusListener);
target.element.removeEventListener('blur', target.domBlurListener);
Copy link
Contributor

@chunhtai chunhtai Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to tell the difference between framework request focus and user interacting dom focus by the Event object in the listener?

Copy link
Contributor Author

@yjbanov yjbanov Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of one for focus/blur events specifically. The closest thing available is the isTrusted property, but it only detects dispatchEvent, which is not enough. Clicks can be differentiated by looking at their screenX/Y properties (source), but focus/blur doesn't have coordinates.

}

void _didReceiveDomFocus() {
Expand All @@ -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.
Expand Down Expand Up @@ -240,6 +278,7 @@ class AccessibilityFocusManager {
return;
}

_lastEvent = AccessibilityFocusManagerEvent.requestedFocus;
target.element.focus();
});
}
Expand Down
105 changes: 66 additions & 39 deletions lib/web_ui/test/engine/semantics/semantics_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
});
}
Expand Down Expand Up @@ -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;
});

Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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
Expand Down