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

[web] Transfer focus to view rootElement on blur/remove. #55045

Merged
merged 9 commits into from
Nov 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion lib/web_ui/lib/src/engine/semantics/semantics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1987,7 +1987,9 @@ class SemanticsObject {
void dispose() {
assert(!_isDisposed);
_isDisposed = true;
element.remove();

EnginePlatformDispatcher.instance.viewManager.safeRemoveSync(element);

_parent = null;
semanticRole?.dispose();
Copy link
Contributor

Choose a reason for hiding this comment

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

Some roles will remove their DOM children that may have focus in their implementation of dispose().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't see any test break. Is this a behavior that can/will be added later? Do I need to change other classes as well?

Apologies, I have zero experience with semantics.

semanticRole = null;
Expand Down
11 changes: 1 addition & 10 deletions lib/web_ui/lib/src/engine/semantics/text_field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy {
}
subscriptions.clear();
lastEditingState = null;

// If the text element still has focus, remove focus from the editable
// element to cause the on-screen keyboard, if any, to hide (e.g. on iOS,
// Android).
// Otherwise, the keyboard stays on screen even when the user navigates to
// a different screen (e.g. by hitting the "back" button).
// 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);
EnginePlatformDispatcher.instance.viewManager.safeBlur(activeDomElement);
domElement = null;
activeTextField = null;
_queuedStyle = null;
Expand Down
27 changes: 2 additions & 25 deletions lib/web_ui/lib/src/engine/text_editing/text_editing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1411,9 +1411,9 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements
inputConfiguration.autofillGroup?.formElement != null) {
_styleAutofillElements(activeDomElement, isOffScreen: true);
inputConfiguration.autofillGroup?.storeForm();
scheduleFocusFlutterView(activeDomElement, activeDomElementView);
EnginePlatformDispatcher.instance.viewManager.safeBlur(activeDomElement);
} else {
scheduleFocusFlutterView(activeDomElement, activeDomElementView, removeElement: true);
EnginePlatformDispatcher.instance.viewManager.safeRemove(activeDomElement);
}
domElement = null;
}
Expand Down Expand Up @@ -1573,29 +1573,6 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements
void moveFocusToActiveDomElement() {
activeDomElement.focusWithoutScroll();
}

/// Move the focus to the given [EngineFlutterView] in the next timer event.
///
/// The timer gives the engine the opportunity to focus on another element.
/// Shifting focus immediately can cause the keyboard to jump.
static void scheduleFocusFlutterView(
DomElement element,
EngineFlutterView? view, {
bool removeElement = false,
}) {
Timer(Duration.zero, () {
// If by the time the timer fired the focused element is no longer the
// editing element whose editing session was disabled, there's no need to
// move the focus, as it is likely that another widget already took the
// focus.
if (element == domDocument.activeElement) {
view?.dom.rootElement.focusWithoutScroll();
}
if (removeElement) {
element.remove();
}
});
}
}

/// IOS/Safari behaviour for text editing.
Expand Down
83 changes: 80 additions & 3 deletions lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,86 @@ class FlutterViewManager {
const String viewRootSelector =
'${DomManager.flutterViewTagName}[${GlobalHtmlAttributes.flutterViewIdAttributeName}]';
final DomElement? viewRoot = element?.closest(viewRootSelector);
final String? viewIdAttribute = viewRoot?.getAttribute(GlobalHtmlAttributes.flutterViewIdAttributeName);
final int? viewId = viewIdAttribute == null ? null : int.parse(viewIdAttribute);
return viewId == null ? null : _viewData[viewId];
if (viewRoot == null) {
// `element` is not inside any flutter view.
return null;
}

final String? viewIdAttribute = viewRoot.getAttribute(GlobalHtmlAttributes.flutterViewIdAttributeName);
assert(viewIdAttribute != null, 'Located Flutter view is missing its id attribute.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the additional checks ✅


final int? viewId = int.tryParse(viewIdAttribute!);
assert(viewId != null, 'Flutter view id must be a valid int.');

return _viewData[viewId];
}

/// Blurs [element] by transferring its focus to its [EngineFlutterView]
/// `rootElement`.
///
/// This operation is asynchronous, but happens as soon as possible
/// (see [Timer.run]).
Future<void> safeBlur(DomElement element) {
return Future<void>(() {
_transferFocusToViewRoot(element);
});
}

/// Removes [element] after transferring its focus to its [EngineFlutterView]
/// `rootElement`.
///
/// This operation is asynchronous, but happens as soon as possible
/// (see [Timer.run]).
///
/// There's a synchronous version of this method: [safeRemoveSync].
Future<void> safeRemove(DomElement element) {
return Future<void>(() => safeRemoveSync(element));
}

/// Synchronously removes [element] after transferring its focus to its
/// [EngineFlutterView] `rootElement`.
///
/// This is the synchronous version of [safeRemove].
void safeRemoveSync(DomElement element) {
_transferFocusToViewRoot(element, removeElement: true);
}

/// Blurs (removes focus) from [element] by transferring its focus to its
/// [EngineFlutterView] DOM's `rootElement` before (optionally) removing it.
///
/// By default, blurring a focused `element` (or removing it from the DOM)
/// transfers its focus to the `body` element of the page.
///
/// This method achieves two things when blurring/removing `element`:
///
/// * Ensures the focus is preserved within the Flutter View when blurring
/// elements that are part of the internal DOM structure of the Flutter
/// app.
/// * Prevents the Flutter engine from reporting bogus "blur" events from the
/// Flutter View.
///
/// When [removeElement] is true, `element` will be removed from the DOM after
/// its focus (or that of any of its children) is transferred to the root of
/// the view.
///
/// See: https://jsfiddle.net/ditman/1e2swpno for a JS focus transfer demo.
void _transferFocusToViewRoot(
DomElement element, {
bool removeElement = false,
}) {
final DomElement? activeElement = domDocument.activeElement;
// If the element we're operating on is not active anymore (it can happen
// when this method is called asynchronously), OR the element that we want
// to remove *contains* the `activeElement`.
if (element == activeElement || removeElement && element.contains(activeElement)) {
// Transfer the browser focus to the `rootElement` of the
// [EngineFlutterView] that contains `element`
final EngineFlutterView? view = findViewForElement(element);
view?.dom.rootElement.focusWithoutScroll();
}
if (removeElement) {
element.remove();
}
}

void dispose() {
Expand Down
5 changes: 4 additions & 1 deletion lib/web_ui/test/engine/semantics/semantics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ EngineSemanticsOwner owner() => EnginePlatformDispatcher.instance.implicitView!.
DomElement get platformViewsHost =>
EnginePlatformDispatcher.instance.implicitView!.dom.platformViewsHost;

DomElement get flutterViewRoot =>
EnginePlatformDispatcher.instance.implicitView!.dom.rootElement;

void main() {
internalBootstrapBrowserTest(() {
return testMain;
Expand Down Expand Up @@ -3567,7 +3570,7 @@ void _testRoute() {
tester.apply();

expect(capturedActions, isEmpty);
expect(domDocument.activeElement, domDocument.body);
expect(domDocument.activeElement, flutterViewRoot);

semantics().semanticsEnabled = false;
});
Expand Down
18 changes: 11 additions & 7 deletions lib/web_ui/test/engine/semantics/text_field_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';
import 'package:ui/src/engine.dart' hide window;
import 'package:ui/ui.dart' as ui;
import 'package:ui/ui_web/src/ui_web.dart' as ui_web;

import '../../common/test_initialization.dart';
import 'semantics_tester.dart';
Expand All @@ -24,8 +23,8 @@ final InputConfiguration multilineConfig = InputConfiguration(
);

EngineSemantics semantics() => EngineSemantics.instance;
EngineSemanticsOwner owner() =>
EnginePlatformDispatcher.instance.implicitView!.semantics;
EngineFlutterView get flutterView => EnginePlatformDispatcher.instance.implicitView!;
EngineSemanticsOwner owner() => flutterView.semantics;

const MethodCodec codec = JSONMethodCodec();

Expand Down Expand Up @@ -88,6 +87,8 @@ void testMain() {

tearDown(() {
semantics().semanticsEnabled = false;
// Most tests in this file expect to start with nothing focused.
domDocument.activeElement?.blur();
});

test('renders a text field', () {
Expand Down Expand Up @@ -156,8 +157,7 @@ void testMain() {

expect(
owner().semanticsHost.ownerDocument?.activeElement, isNot(textField));
// TODO(yjbanov): https://github.com/flutter/flutter/issues/46638
}, skip: ui_web.browser.browserEngine == ui_web.BrowserEngine.firefox);
});

test('Syncs semantic state from framework', () async {
expect(
Expand Down Expand Up @@ -226,7 +226,9 @@ void testMain() {
await Future<void>.delayed(Duration.zero);
expect(
owner().semanticsHost.ownerDocument?.activeElement,
EnginePlatformDispatcher.instance.implicitView!.dom.rootElement,
flutterView.dom.rootElement,
reason: 'Focus should be returned to the root element of the Flutter view '
'after housekeeping DOM operations (blur/remove)',
);

// There was no user interaction with the <input> element,
Expand Down Expand Up @@ -367,7 +369,9 @@ void testMain() {
await Future<void>.delayed(Duration.zero);
expect(
owner().semanticsHost.ownerDocument?.activeElement,
EnginePlatformDispatcher.instance.implicitView!.dom.rootElement,
flutterView.dom.rootElement,
reason: 'Focus should be returned to the root element of the Flutter view '
'after housekeeping DOM operations (blur/remove)',
);
});

Expand Down
4 changes: 1 addition & 3 deletions lib/web_ui/test/engine/surface/scene_builder_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import 'package:test/bootstrap/browser.dart';
import 'package:test/test.dart';
import 'package:ui/src/engine.dart';
import 'package:ui/ui.dart' as ui;
import 'package:ui/ui_web/src/ui_web.dart' as ui_web;

import '../../common/matchers.dart';
import '../../common/rendering.dart';
Expand Down Expand Up @@ -182,8 +181,7 @@ void testMain() {
expect(picture.buildCount, 1);
expect(picture.updateCount, 0);
expect(picture.applyPaintCount, 2);
}, // TODO(yjbanov): https://github.com/flutter/flutter/issues/46638
skip: ui_web.browser.browserEngine == ui_web.BrowserEngine.firefox);
});
});

group('Compositing order', () {
Expand Down