Flutter views can gain focus (flutter/engine#54985)

I am unsure why the `tabindex` was removed when semantics were enabled. It seems this change was made without a clear explanation (by me). This PR shouldn't cause any issues as Flutter Views already have a tabindex, we're not adding a new one. This change is necessary because the semantics text strategy refocuses the view on deactivation, requiring the Flutter view to be focusable.

ThIs PR is a requirement to enable https://github.com/flutter/engine/pull/54966. 

https://github.com/flutter/flutter/issues/153022

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
Juanjo Tugores 2024-11-18 12:26:13 -08:00 committed by GitHub
parent f20a08ea1f
commit 04f26f4377
2 changed files with 33 additions and 49 deletions

View File

@ -27,18 +27,20 @@ final class ViewFocusBinding {
StreamSubscription<int>? _onViewCreatedListener;
void init() {
// We need a global listener here to know if the user was pressing "shift"
// when the Flutter view receives focus, to move the Flutter focus to the
// *last* focusable element.
domDocument.body?.addEventListener(_keyDown, _handleKeyDown);
domDocument.body?.addEventListener(_keyUp, _handleKeyUp);
domDocument.body?.addEventListener(_focusin, _handleFocusin);
domDocument.body?.addEventListener(_focusout, _handleFocusout);
// If so, update `_handleViewCreated` and add a `_handleViewDisposed` to attach
// and remove the focus/blur listener.
_onViewCreatedListener = _viewManager.onViewCreated.listen(_handleViewCreated);
}
void dispose() {
domDocument.body?.removeEventListener(_keyDown, _handleKeyDown);
domDocument.body?.removeEventListener(_keyUp, _handleKeyUp);
domDocument.body?.removeEventListener(_focusin, _handleFocusin);
domDocument.body?.removeEventListener(_focusout, _handleFocusout);
_onViewCreatedListener?.cancel();
}
@ -48,12 +50,13 @@ final class ViewFocusBinding {
}
final DomElement? viewElement = _viewManager[viewId]?.dom.rootElement;
if (state == ui.ViewFocusState.focused) {
switch (state) {
case ui.ViewFocusState.focused:
// Only move the focus to the flutter view if nothing inside it is focused already.
if (viewId != _viewId(domDocument.activeElement)) {
viewElement?.focusWithoutScroll();
}
} else {
case ui.ViewFocusState.unfocused:
viewElement?.blur();
}
}
@ -115,8 +118,8 @@ final class ViewFocusBinding {
direction: _viewFocusDirection,
);
}
_maybeMarkViewAsFocusable(_lastViewId, reachableByKeyboard: true);
_maybeMarkViewAsFocusable(viewId, reachableByKeyboard: false);
_updateViewKeyboardReachability(_lastViewId, reachable: true);
_updateViewKeyboardReachability(viewId, reachable: false);
_lastViewId = viewId;
_onViewFocusChange(event);
}
@ -127,29 +130,32 @@ final class ViewFocusBinding {
}
void _handleViewCreated(int viewId) {
_maybeMarkViewAsFocusable(viewId, reachableByKeyboard: true);
final DomElement? rootElement = _viewManager[viewId]?.dom.rootElement;
rootElement?.addEventListener(_focusin, _handleFocusin);
rootElement?.addEventListener(_focusout, _handleFocusout);
_updateViewKeyboardReachability(viewId, reachable: true);
}
void _maybeMarkViewAsFocusable(
// Controls whether the Flutter view identified by [viewId] is reachable by
// keyboard.
void _updateViewKeyboardReachability(
int? viewId, {
required bool reachableByKeyboard,
required bool reachable,
}) {
if (viewId == null) {
return;
}
final DomElement? rootElement = _viewManager[viewId]?.dom.rootElement;
if (EngineSemantics.instance.semanticsEnabled) {
rootElement?.removeAttribute('tabindex');
} else {
// A tabindex with value zero means the DOM element can be reached by using
// the keyboard (tab, shift + tab). When its value is -1 it is still focusable
// but can't be focused by the result of keyboard events This is specially
// A tabindex with value zero means the DOM element can be reached using the
// keyboard (tab, shift + tab). When its value is -1 it is still focusable
// but can't be focused as the result of keyboard events. This is specially
// important when the semantics tree is enabled as it puts DOM nodes inside
// the flutter view and having it with a zero tabindex messes the focus
// traversal order when pressing tab or shift tab.
rootElement?.setAttribute('tabindex', reachableByKeyboard ? 0 : -1);
}
rootElement?.setAttribute('tabindex', reachable ? 0 : -1);
}
static const String _focusin = 'focusin';

View File

@ -68,28 +68,6 @@ void testMain() {
expect(view2.dom.rootElement.getAttribute('tabindex'), '0');
});
test('never marks the views as focusable with semantincs enabled', () async {
EngineSemantics.instance.semanticsEnabled = true;
final EngineFlutterView view1 = createAndRegisterView(dispatcher);
final EngineFlutterView view2 = createAndRegisterView(dispatcher);
expect(view1.dom.rootElement.getAttribute('tabindex'), isNull);
expect(view2.dom.rootElement.getAttribute('tabindex'), isNull);
view1.dom.rootElement.focusWithoutScroll();
expect(view1.dom.rootElement.getAttribute('tabindex'), isNull);
expect(view2.dom.rootElement.getAttribute('tabindex'), isNull);
view2.dom.rootElement.focusWithoutScroll();
expect(view1.dom.rootElement.getAttribute('tabindex'), isNull);
expect(view2.dom.rootElement.getAttribute('tabindex'), isNull);
view2.dom.rootElement.blur();
expect(view1.dom.rootElement.getAttribute('tabindex'), isNull);
expect(view2.dom.rootElement.getAttribute('tabindex'), isNull);
});
test('fires a focus event - a view was focused', () async {
final EngineFlutterView view = createAndRegisterView(dispatcher);