diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index ea31b6356e3b1..b3f59d1292ecb 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -1987,7 +1987,9 @@ class SemanticsObject { void dispose() { assert(!_isDisposed); _isDisposed = true; - element.remove(); + + EnginePlatformDispatcher.instance.viewManager.safeRemoveSync(element); + _parent = null; semanticRole?.dispose(); semanticRole = null; diff --git a/lib/web_ui/lib/src/engine/semantics/text_field.dart b/lib/web_ui/lib/src/engine/semantics/text_field.dart index 1ad9cecf839bb..0bf734b0797df 100644 --- a/lib/web_ui/lib/src/engine/semantics/text_field.dart +++ b/lib/web_ui/lib/src/engine/semantics/text_field.dart @@ -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; diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index e5082a09e25af..edba5f73afe7c 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -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; } @@ -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. diff --git a/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart b/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart index 685e940a53cf1..16a16d5192781 100644 --- a/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart +++ b/lib/web_ui/lib/src/engine/view_embedder/flutter_view_manager.dart @@ -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.'); + + 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 safeBlur(DomElement element) { + return Future(() { + _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 safeRemove(DomElement element) { + return Future(() => 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() { diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index dd9cf23b9dea7..8a24eb38fb3c3 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -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; @@ -3567,7 +3570,7 @@ void _testRoute() { tester.apply(); expect(capturedActions, isEmpty); - expect(domDocument.activeElement, domDocument.body); + expect(domDocument.activeElement, flutterViewRoot); semantics().semanticsEnabled = false; }); diff --git a/lib/web_ui/test/engine/semantics/text_field_test.dart b/lib/web_ui/test/engine/semantics/text_field_test.dart index c93dc3bf4a86f..38cef56fa80e3 100644 --- a/lib/web_ui/test/engine/semantics/text_field_test.dart +++ b/lib/web_ui/test/engine/semantics/text_field_test.dart @@ -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'; @@ -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(); @@ -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', () { @@ -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( @@ -226,7 +226,9 @@ void testMain() { await Future.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 element, @@ -367,7 +369,9 @@ void testMain() { await Future.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)', ); }); diff --git a/lib/web_ui/test/engine/surface/scene_builder_test.dart b/lib/web_ui/test/engine/surface/scene_builder_test.dart index d2c47c17d8339..a058051b4017f 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -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'; @@ -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', () {