From 34bb652f5949e1223fc5f5e0c01bd752b04c8762 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 15 Dec 2025 14:22:14 -0500 Subject: [PATCH] [web] Fix `resizeToAvoidBottomInset` on Android web (#179581) Fixes https://github.com/flutter/flutter/issues/175074 There was an implicit expectation in the resizing code: When the virtual keyboard is up on mobile, the view's `physicalSize` remains fixed, and any resizes (presumably caused by the virtual keyboard itself) should be considered as `viewInsets`. This expectation was broken inadvertently by my PR: https://github.com/flutter/flutter/pull/172493
The sequence of events that lead to the reported issue: 1. View's physical size is calculated based on window size. 2. Text editing starts, virtual keyboard comes up. 3. View's physical size remains unchanged, and the difference caused by the keyboard is reported as view insets to the framework. 4. (so far so good). 5. When `resizeToAvoidBottomInset` is true, the framework will re-render its content to fit the available size. 6. The new render call comes with a new size that's basically `physical height - bottom inset`. 7. The engine takes that new size and applies it to the DOM. 8. (now the view's DOM element has been resized incorrectly). 9. ... 10. Eventually, this leads to the new insets being calculated incorrectly resulting in negative insets which cause the framework to throw.
The fix involves the following: 1. Respect the expectation mentioned above: when the keyboard is up, view's physical size should remain unchanged. 2. *_If_* we ever end up with negative insets, catch it earlier in the engine so it's easier to debug the root cause. 3. New regression test. --- .../lib/web_ui/lib/src/engine/window.dart | 54 ++++++++++--- .../lib/web_ui/test/engine/window_test.dart | 78 ++++++++++++------- 2 files changed, 94 insertions(+), 38 deletions(-) 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); + }); + }); }