diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart index dc9fae6631a..71986b03bc9 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart @@ -69,7 +69,7 @@ class EngineFlutterView implements ui.FlutterView { // hot restart. embeddingStrategy.attachViewRoot(dom.rootElement); pointerBinding = PointerBinding(this); - _resizeSubscription = onResize.listen(_didResize); + _resizeSubscription = onResize.listen(_handleBrowserResize); _globalHtmlAttributes.applyAttributes( viewId: viewId, rendererTag: renderer.rendererTag, @@ -120,7 +120,7 @@ class EngineFlutterView implements ui.FlutterView { void render(ui.Scene scene, {ui.Size? size}) { assert(!isDisposed, 'Trying to render a disposed EngineFlutterView.'); if (size != null) { - resize(size); + handleFrameworkResize(size); } platformDispatcher.render(scene, this); } @@ -208,15 +208,27 @@ class EngineFlutterView implements ui.FlutterView { /// hiding `overflow`). Flutter does not attempt to interpret the styles of /// `hostElement` to compute its `physicalConstraints`, only its current size. @visibleForTesting - void resize(ui.Size newPhysicalSize) { + void handleFrameworkResize(ui.Size newPhysicalSize) { + // TODO(mdebbar): This resizing is only needed for the multiview mode. Should we make it a no-op + // in the full page mode to avoid doing unnecessary work? + // The browser uses CSS, and CSS operates in logical sizes. final ui.Size logicalSize = newPhysicalSize / devicePixelRatio; dom.rootElement.style ..width = '${logicalSize.width}px' ..height = '${logicalSize.height}px'; - // Force an update of the physicalSize so it's ready for the renderer. - _physicalSize = _computePhysicalSize(); + // When the keyboard is active on mobile, we explicitly do not update + // `_physicalSize`. This is because `_handleBrowserResize` (the method + // that handles browser-initiated resizes) has special logic to + // keep `_physicalSize` stale (large) and instead rely on `viewInsets` + // to shrink the visible area. If `_handleFrameworkResize` were to update `_physicalSize` + // to the smaller visible size, it would break the `viewInsets` logic + // on subsequent calculations. + if (!_shouldPreservePhysicalSizeOnResize) { + // Force an update of the physicalSize so it's ready for the renderer. + _physicalSize = _computePhysicalSize(); + } } /// Lazily populated and cleared at the end of the frame. @@ -274,7 +286,14 @@ class EngineFlutterView implements ui.FlutterView { Stream get onResize => dimensionsProvider.onResize; - /// Called immediately after the view has been resized. + /// Whether to preserve the physical size of the view when the browser window + /// resizes. + /// + /// This is used to prevent the view from resizing when the on-screen keyboard + /// appears on mobile devices. + bool get _shouldPreservePhysicalSizeOnResize => isMobile && textEditing.isEditing; + + /// Called immediately after the view has been resized by the browser. /// /// When there is a text editing going on in mobile devices, do not change /// the physicalSize, change the [window.viewInsets]. See: @@ -283,12 +302,17 @@ class EngineFlutterView implements ui.FlutterView { /// /// Note: always check for rotations for a mobile device. Update the physical /// size if the change is caused by a rotation. - void _didResize(ui.Size? newSize) { + /// + /// When `_shouldPreservePhysicalSizeOnResize` is true (i.e., keyboard is active + /// on mobile), `_physicalSize` is deliberately kept stale (representing the + /// full screen size) while `viewInsets` are updated to reflect the keyboard's + /// presence. This allows the framework to correctly shrink its content using + /// `resizeToAvoidBottomInset`. When the keyboard is dismissed, `_physicalSize` + /// is updated to the actual new physical size of the window. + void _handleBrowserResize(ui.Size? _) { StyleManager.scaleSemanticsHost(dom.semanticsHost, devicePixelRatio); final ui.Size newPhysicalSize = _computePhysicalSize(); - final bool isEditingOnMobile = - isMobile && !_isRotation(newPhysicalSize) && textEditing.isEditing; - if (isEditingOnMobile) { + if (_shouldPreservePhysicalSizeOnResize && !_isRotation(newPhysicalSize)) { _computeOnScreenKeyboardInsets(true); } else { _physicalSize = newPhysicalSize; @@ -338,6 +362,13 @@ class EngineFlutterView implements ui.FlutterView { _physicalSize!.height, isEditingOnMobile, ); + + // Ensure that viewInsets are never negative. If it's not caught here, it will be caught later + // in the framework, and will be hard to debug since the root cause is here. + assert( + _viewInsets.isNonNegative, + 'ViewInsets cannot be negative. This is usually caused by an incorrect physicalSize calculation when the keyboard is being dismissed.', + ); } } @@ -707,6 +738,9 @@ class ViewPadding implements ui.ViewPadding { final double right; @override final double bottom; + + /// Returns true if all padding values are non-negative. + bool get isNonNegative => left >= 0.0 && top >= 0.0 && right >= 0.0 && bottom >= 0.0; } class ViewConstraints implements ui.ViewConstraints { diff --git a/engine/src/flutter/lib/web_ui/test/engine/window_test.dart b/engine/src/flutter/lib/web_ui/test/engine/window_test.dart index b721110aea9..a17e4a32cc1 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/window_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/window_test.dart @@ -11,6 +11,7 @@ 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/test_initialization.dart'; @@ -25,7 +26,7 @@ void main() { internalBootstrapBrowserTest(() => testMain); } -Future testMain() async { +void testMain() { setUpImplicitView(); test('onTextScaleFactorChanged preserves the zone', () { @@ -520,33 +521,30 @@ Future testMain() async { domWindow['screen'] = original; }); - test( - 'SingletonFlutterWindow implements locale, locales, and locale change notifications', - () async { - // This will count how many times we notified about locale changes. - var localeChangedCount = 0; - myWindow.onLocaleChanged = () { - localeChangedCount += 1; - }; + test('SingletonFlutterWindow implements locale, locales, and locale change notifications', () { + // This will count how many times we notified about locale changes. + var localeChangedCount = 0; + myWindow.onLocaleChanged = () { + localeChangedCount += 1; + }; - // We populate the initial list of locales automatically (only test that we - // got some locales; some contributors may be in different locales, so we - // can't test the exact contents). - expect(myWindow.locale, isA()); - expect(myWindow.locales, isNotEmpty); + // We populate the initial list of locales automatically (only test that we + // got some locales; some contributors may be in different locales, so we + // can't test the exact contents). + expect(myWindow.locale, isA()); + expect(myWindow.locales, isNotEmpty); - // Trigger a change notification (reset locales because the notification - // doesn't actually change the list of languages; the test only observes - // that the list is populated again). - EnginePlatformDispatcher.instance.debugResetLocales(); - expect(myWindow.locales, isEmpty); - expect(myWindow.locale, equals(const ui.Locale.fromSubtags())); - expect(localeChangedCount, 0); - domWindow.dispatchEvent(createDomEvent('Event', 'languagechange')); - expect(myWindow.locales, isNotEmpty); - expect(localeChangedCount, 1); - }, - ); + // Trigger a change notification (reset locales because the notification + // doesn't actually change the list of languages; the test only observes + // that the list is populated again). + EnginePlatformDispatcher.instance.debugResetLocales(); + expect(myWindow.locales, isEmpty); + expect(myWindow.locale, equals(const ui.Locale.fromSubtags())); + expect(localeChangedCount, 0); + domWindow.dispatchEvent(createDomEvent('Event', 'languagechange')); + expect(myWindow.locales, isNotEmpty); + expect(localeChangedCount, 1); + }); test('dispatches browser event on flutter/service_worker channel', () async { final completer = Completer(); @@ -741,7 +739,7 @@ Future testMain() async { ..height = 'auto'; // Resize the host to 20x20 (physical pixels). - view.resize(const ui.Size.square(50)); + view.handleFrameworkResize(const ui.Size.square(50)); // The view's physicalSize should be updated too. expect(view.physicalSize, const ui.Size(50.0, 50.0)); @@ -775,7 +773,7 @@ Future testMain() async { EngineFlutterDisplay.instance.debugOverrideDevicePixelRatio(null); }); - test('JsViewConstraints are passed and used to compute physicalConstraints', () async { + test('JsViewConstraints are passed and used to compute physicalConstraints', () { view = EngineFlutterView( EnginePlatformDispatcher.instance, host, @@ -799,4 +797,28 @@ Future testMain() async { ); }); }); + + group('keyboard resize behavior', () { + setUp(() { + // Simulate keyboard being up. + textEditing.isEditing = true; + ui_web.browser.debugOperatingSystemOverride = ui_web.OperatingSystem.android; + }); + + tearDown(() { + textEditing.isEditing = false; + ui_web.browser.debugOperatingSystemOverride = null; + }); + + test('physicalSize remains unchanged when keyboard is up', () { + final ui.Size initialPhysicalSize = myWindow.physicalSize; + + // Pick a smaller size. + final ui.Size newSize = initialPhysicalSize ~/ 2; + myWindow.handleFrameworkResize(newSize); + + // View's `physicalSize` should remain unchanged. + expect(myWindow.physicalSize, initialPhysicalSize); + }); + }); }