Skip to content

Commit b78aaca

Browse files
authored
[web] Transfer focus to view rootElement on blur/remove. (flutter/engine#55045)
The `safeBlur`/`safeRemove`/`safeRemoveSync` methods in the view manager should become the standard way to "blur" and "remove" elements within the web engine. Calling these method ensures the blur operation doesn't disrupt the framework's view focus management because these methods transfer the focus from the current element to its containing EngineFlutterView's `rootElement`, so focus never abandons the Flutter view unless the user wants to. This is a generalization of the former `DefaultTextEditingStrategy.scheduleFocusFlutterView`, which turns out is needed in anything that touches elements that may receive focus in the engine, not just text editing. ## Issue (Maybe) Part of flutter#157387 (Opportunistically) Fixes flutter#46638 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 6066418 commit b78aaca

File tree

7 files changed

+102
-50
lines changed

7 files changed

+102
-50
lines changed

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1987,7 +1987,9 @@ class SemanticsObject {
19871987
void dispose() {
19881988
assert(!_isDisposed);
19891989
_isDisposed = true;
1990-
element.remove();
1990+
1991+
EnginePlatformDispatcher.instance.viewManager.safeRemoveSync(element);
1992+
19911993
_parent = null;
19921994
semanticRole?.dispose();
19931995
semanticRole = null;

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy {
114114
}
115115
subscriptions.clear();
116116
lastEditingState = null;
117-
118-
// If the text element still has focus, remove focus from the editable
119-
// element to cause the on-screen keyboard, if any, to hide (e.g. on iOS,
120-
// Android).
121-
// Otherwise, the keyboard stays on screen even when the user navigates to
122-
// a different screen (e.g. by hitting the "back" button).
123-
// Keep this consistent with how DefaultTextEditingStrategy does it. As of
124-
// right now, the only difference is that semantic text fields do not
125-
// participate in form autofill.
126-
DefaultTextEditingStrategy.scheduleFocusFlutterView(activeDomElement, activeDomElementView);
117+
EnginePlatformDispatcher.instance.viewManager.safeBlur(activeDomElement);
127118
domElement = null;
128119
activeTextField = null;
129120
_queuedStyle = null;

engine/src/flutter/lib/web_ui/lib/src/engine/text_editing/text_editing.dart

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1411,9 +1411,9 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements
14111411
inputConfiguration.autofillGroup?.formElement != null) {
14121412
_styleAutofillElements(activeDomElement, isOffScreen: true);
14131413
inputConfiguration.autofillGroup?.storeForm();
1414-
scheduleFocusFlutterView(activeDomElement, activeDomElementView);
1414+
EnginePlatformDispatcher.instance.viewManager.safeBlur(activeDomElement);
14151415
} else {
1416-
scheduleFocusFlutterView(activeDomElement, activeDomElementView, removeElement: true);
1416+
EnginePlatformDispatcher.instance.viewManager.safeRemove(activeDomElement);
14171417
}
14181418
domElement = null;
14191419
}
@@ -1573,29 +1573,6 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements
15731573
void moveFocusToActiveDomElement() {
15741574
activeDomElement.focusWithoutScroll();
15751575
}
1576-
1577-
/// Move the focus to the given [EngineFlutterView] in the next timer event.
1578-
///
1579-
/// The timer gives the engine the opportunity to focus on another element.
1580-
/// Shifting focus immediately can cause the keyboard to jump.
1581-
static void scheduleFocusFlutterView(
1582-
DomElement element,
1583-
EngineFlutterView? view, {
1584-
bool removeElement = false,
1585-
}) {
1586-
Timer(Duration.zero, () {
1587-
// If by the time the timer fired the focused element is no longer the
1588-
// editing element whose editing session was disabled, there's no need to
1589-
// move the focus, as it is likely that another widget already took the
1590-
// focus.
1591-
if (element == domDocument.activeElement) {
1592-
view?.dom.rootElement.focusWithoutScroll();
1593-
}
1594-
if (removeElement) {
1595-
element.remove();
1596-
}
1597-
});
1598-
}
15991576
}
16001577

16011578
/// IOS/Safari behaviour for text editing.

engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,86 @@ class FlutterViewManager {
108108
const String viewRootSelector =
109109
'${DomManager.flutterViewTagName}[${GlobalHtmlAttributes.flutterViewIdAttributeName}]';
110110
final DomElement? viewRoot = element?.closest(viewRootSelector);
111-
final String? viewIdAttribute = viewRoot?.getAttribute(GlobalHtmlAttributes.flutterViewIdAttributeName);
112-
final int? viewId = viewIdAttribute == null ? null : int.parse(viewIdAttribute);
113-
return viewId == null ? null : _viewData[viewId];
111+
if (viewRoot == null) {
112+
// `element` is not inside any flutter view.
113+
return null;
114+
}
115+
116+
final String? viewIdAttribute = viewRoot.getAttribute(GlobalHtmlAttributes.flutterViewIdAttributeName);
117+
assert(viewIdAttribute != null, 'Located Flutter view is missing its id attribute.');
118+
119+
final int? viewId = int.tryParse(viewIdAttribute!);
120+
assert(viewId != null, 'Flutter view id must be a valid int.');
121+
122+
return _viewData[viewId];
123+
}
124+
125+
/// Blurs [element] by transferring its focus to its [EngineFlutterView]
126+
/// `rootElement`.
127+
///
128+
/// This operation is asynchronous, but happens as soon as possible
129+
/// (see [Timer.run]).
130+
Future<void> safeBlur(DomElement element) {
131+
return Future<void>(() {
132+
_transferFocusToViewRoot(element);
133+
});
134+
}
135+
136+
/// Removes [element] after transferring its focus to its [EngineFlutterView]
137+
/// `rootElement`.
138+
///
139+
/// This operation is asynchronous, but happens as soon as possible
140+
/// (see [Timer.run]).
141+
///
142+
/// There's a synchronous version of this method: [safeRemoveSync].
143+
Future<void> safeRemove(DomElement element) {
144+
return Future<void>(() => safeRemoveSync(element));
145+
}
146+
147+
/// Synchronously removes [element] after transferring its focus to its
148+
/// [EngineFlutterView] `rootElement`.
149+
///
150+
/// This is the synchronous version of [safeRemove].
151+
void safeRemoveSync(DomElement element) {
152+
_transferFocusToViewRoot(element, removeElement: true);
153+
}
154+
155+
/// Blurs (removes focus) from [element] by transferring its focus to its
156+
/// [EngineFlutterView] DOM's `rootElement` before (optionally) removing it.
157+
///
158+
/// By default, blurring a focused `element` (or removing it from the DOM)
159+
/// transfers its focus to the `body` element of the page.
160+
///
161+
/// This method achieves two things when blurring/removing `element`:
162+
///
163+
/// * Ensures the focus is preserved within the Flutter View when blurring
164+
/// elements that are part of the internal DOM structure of the Flutter
165+
/// app.
166+
/// * Prevents the Flutter engine from reporting bogus "blur" events from the
167+
/// Flutter View.
168+
///
169+
/// When [removeElement] is true, `element` will be removed from the DOM after
170+
/// its focus (or that of any of its children) is transferred to the root of
171+
/// the view.
172+
///
173+
/// See: https://jsfiddle.net/ditman/1e2swpno for a JS focus transfer demo.
174+
void _transferFocusToViewRoot(
175+
DomElement element, {
176+
bool removeElement = false,
177+
}) {
178+
final DomElement? activeElement = domDocument.activeElement;
179+
// If the element we're operating on is not active anymore (it can happen
180+
// when this method is called asynchronously), OR the element that we want
181+
// to remove *contains* the `activeElement`.
182+
if (element == activeElement || removeElement && element.contains(activeElement)) {
183+
// Transfer the browser focus to the `rootElement` of the
184+
// [EngineFlutterView] that contains `element`
185+
final EngineFlutterView? view = findViewForElement(element);
186+
view?.dom.rootElement.focusWithoutScroll();
187+
}
188+
if (removeElement) {
189+
element.remove();
190+
}
114191
}
115192

116193
void dispose() {

engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ EngineSemanticsOwner owner() => EnginePlatformDispatcher.instance.implicitView!.
2525
DomElement get platformViewsHost =>
2626
EnginePlatformDispatcher.instance.implicitView!.dom.platformViewsHost;
2727

28+
DomElement get flutterViewRoot =>
29+
EnginePlatformDispatcher.instance.implicitView!.dom.rootElement;
30+
2831
void main() {
2932
internalBootstrapBrowserTest(() {
3033
return testMain;
@@ -3567,7 +3570,7 @@ void _testRoute() {
35673570
tester.apply();
35683571

35693572
expect(capturedActions, isEmpty);
3570-
expect(domDocument.activeElement, domDocument.body);
3573+
expect(domDocument.activeElement, flutterViewRoot);
35713574

35723575
semantics().semanticsEnabled = false;
35733576
});

engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import 'package:test/bootstrap/browser.dart';
99
import 'package:test/test.dart';
1010
import 'package:ui/src/engine.dart' hide window;
1111
import 'package:ui/ui.dart' as ui;
12-
import 'package:ui/ui_web/src/ui_web.dart' as ui_web;
1312

1413
import '../../common/test_initialization.dart';
1514
import 'semantics_tester.dart';
@@ -24,8 +23,8 @@ final InputConfiguration multilineConfig = InputConfiguration(
2423
);
2524

2625
EngineSemantics semantics() => EngineSemantics.instance;
27-
EngineSemanticsOwner owner() =>
28-
EnginePlatformDispatcher.instance.implicitView!.semantics;
26+
EngineFlutterView get flutterView => EnginePlatformDispatcher.instance.implicitView!;
27+
EngineSemanticsOwner owner() => flutterView.semantics;
2928

3029
const MethodCodec codec = JSONMethodCodec();
3130

@@ -88,6 +87,8 @@ void testMain() {
8887

8988
tearDown(() {
9089
semantics().semanticsEnabled = false;
90+
// Most tests in this file expect to start with nothing focused.
91+
domDocument.activeElement?.blur();
9192
});
9293

9394
test('renders a text field', () {
@@ -156,8 +157,7 @@ void testMain() {
156157

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

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

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

engine/src/flutter/lib/web_ui/test/engine/surface/scene_builder_test.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import 'package:test/bootstrap/browser.dart';
1313
import 'package:test/test.dart';
1414
import 'package:ui/src/engine.dart';
1515
import 'package:ui/ui.dart' as ui;
16-
import 'package:ui/ui_web/src/ui_web.dart' as ui_web;
1716

1817
import '../../common/matchers.dart';
1918
import '../../common/rendering.dart';
@@ -182,8 +181,7 @@ void testMain() {
182181
expect(picture.buildCount, 1);
183182
expect(picture.updateCount, 0);
184183
expect(picture.applyPaintCount, 2);
185-
}, // TODO(yjbanov): https://github.com/flutter/flutter/issues/46638
186-
skip: ui_web.browser.browserEngine == ui_web.BrowserEngine.firefox);
184+
});
187185
});
188186

189187
group('Compositing order', () {

0 commit comments

Comments
 (0)