diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 2b96a1184ac..f11e50999f1 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -28,7 +28,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { EnginePlatformDispatcher() { _addBrightnessMediaQueryListener(); HighContrastSupport.instance.addListener(_updateHighContrast); - _addFontSizeObserver(); _addTypographySettingsObserver(); _addLocaleChangedListener(); registerHotRestartListener(dispose); @@ -80,7 +79,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { void dispose() { _removeBrightnessMediaQueryListener(); - _disconnectFontSizeObserver(); _disconnectTypographySettingsObserver(); _removeLocaleChangedListener(); HighContrastSupport.instance.removeListener(_updateHighContrast); @@ -995,52 +993,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { @override bool get alwaysUse24HourFormat => configuration.alwaysUse24HourFormat; - /// Updates [textScaleFactor] and invokes [onTextScaleFactorChanged] and - /// [onPlatformConfigurationChanged] callbacks if [textScaleFactor] changed. - void _updateTextScaleFactor(double value) { - if (configuration.textScaleFactor != value) { - configuration = configuration.copyWith(textScaleFactor: value); - invokeOnPlatformConfigurationChanged(); - invokeOnTextScaleFactorChanged(); - } - } - - /// Watches for font-size changes in the browser's element to - /// recalculate [textScaleFactor]. - /// - /// Updates [textScaleFactor] with the new value. - DomMutationObserver? _fontSizeObserver; - - /// Set the callback function for updating [textScaleFactor] based on - /// font-size changes in the browser's element. - void _addFontSizeObserver() { - const styleAttribute = 'style'; - - _fontSizeObserver = createDomMutationObserver(( - JSArray mutations, - DomMutationObserver _, - ) { - for (final JSAny? mutation in mutations.toDart) { - final record = mutation! as DomMutationRecord; - if (record.type == 'attributes' && record.attributeName == styleAttribute) { - final double newTextScaleFactor = findBrowserTextScaleFactor(); - _updateTextScaleFactor(newTextScaleFactor); - } - } - }); - _fontSizeObserver!.observe( - domDocument.documentElement!, - attributes: true, - attributeFilter: [styleAttribute], - ); - } - - /// Remove the observer for font-size changes in the browser's element. - void _disconnectFontSizeObserver() { - _fontSizeObserver?.disconnect(); - _fontSizeObserver = null; - } - /// Watches for resize changes on an off-screen invisible element to /// recalculate [lineHeightScaleFactorOverride], [letterSpacingOverride], /// [wordSpacingOverride], and [paragraphSpacingOverride]. @@ -1050,6 +1002,16 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { DomResizeObserver? _typographySettingsObserver; DomElement? _typographyMeasurementElement; + /// Updates [textScaleFactor] and returns true if [textScaleFactor] changed. + /// If not then returns false. + bool _updateTextScaleFactor(double value) { + if (configuration.textScaleFactor != value) { + configuration = configuration.apply(textScaleFactor: value); + return true; + } + return false; + } + /// Updates [lineHeightScaleFactorOverride] and return true if /// [lineHeightScaleFactorOverride] changed. If not then returns false. bool _updateLineHeightScaleFactorOverride(double? value) { @@ -1090,9 +1052,10 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { return false; } - /// Set the callback function for updating [lineHeightScaleFactorOverride], - /// [letterSpacingOverride], [wordSpacingOverride], and [paragraphSpacingOverride] - /// based on the sizing changes of an off-screen element with text. + /// Sets the callback function for updating [textScaleFactor], + /// [lineHeightScaleFactorOverride], [letterSpacingOverride], + /// [wordSpacingOverride], and [paragraphSpacingOverride] based on the sizing + /// changes of an off-screen element with text. void _addTypographySettingsObserver() { _typographyMeasurementElement = createDomHTMLParagraphElement(); _typographyMeasurementElement!.text = 'flutter typography measurement'; @@ -1125,12 +1088,14 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { List entries, DomResizeObserver observer, ) { + final double computedTextScaleFactor = findBrowserTextScaleFactor(); final double? lineHeight = parseNumericStyleProperty( _typographyMeasurementElement!, 'line-height', )?.toDouble(); final double? fontSize = parseFontSize(_typographyMeasurementElement!)?.toDouble(); - final double? computedLineHeightScaleFactor = fontSize != null && lineHeight != null + final double? computedLineHeightScaleFactor = + fontSize != null && lineHeight != null && lineHeight != spacingDefault ? lineHeight / fontSize : null; final double? computedWordSpacing = parseNumericStyleProperty( @@ -1150,11 +1115,13 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { 'margin-bottom', )?.toDouble(); + var computedTextScaleFactorChanged = false; var computedLineHeightScaleFactorChanged = false; var computedLetterSpacingChanged = false; var computedWordSpacingChanged = false; var computedParagraphSpacingChanged = false; + computedTextScaleFactorChanged = _updateTextScaleFactor(computedTextScaleFactor); computedLineHeightScaleFactorChanged = _updateLineHeightScaleFactorOverride( computedLineHeightScaleFactor == defaultLineHeightFactor ? null @@ -1170,11 +1137,23 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { computedParagraphSpacing == spacingDefault ? null : computedParagraphSpacing, ); - if (computedLineHeightScaleFactorChanged || + final bool metricsChanged = + computedLineHeightScaleFactorChanged || computedLetterSpacingChanged || computedWordSpacingChanged || - computedParagraphSpacingChanged) { - invokeOnPlatformConfigurationChanged(); + computedParagraphSpacingChanged; + + if (!computedTextScaleFactorChanged && !metricsChanged) { + return; + } + + invokeOnPlatformConfigurationChanged(); + + if (computedTextScaleFactorChanged) { + invokeOnTextScaleFactorChanged(); + } + + if (metricsChanged) { invokeOnMetricsChanged(); } }); diff --git a/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart b/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart index 3816c20917e..a1b19ab0b6a 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart @@ -271,6 +271,7 @@ void testMain() { expect(findBrowserTextScaleFactor(), 1.0); }); + // Regression test for https://github.com/flutter/flutter/issues/178271. test("calls onTextScaleFactorChanged when the element's font-size changes", () async { final DomElement root = domDocument.documentElement!; final String oldFontSize = root.style.fontSize; @@ -281,6 +282,15 @@ void testMain() { ui.PlatformDispatcher.instance.onTextScaleFactorChanged = oldCallback; }); + // Wait for next frame. + Future waitForResizeObserver() { + final completer = Completer(); + domWindow.requestAnimationFrame((_) { + Timer.run(completer.complete); + }); + return completer.future; + } + root.style.fontSize = '16px'; var isCalled = false; @@ -289,18 +299,18 @@ void testMain() { }; root.style.fontSize = '20px'; - await Future.delayed(Duration.zero); + await waitForResizeObserver(); expect(root.style.fontSize, '20px'); expect(isCalled, isTrue); - expect(ui.PlatformDispatcher.instance.textScaleFactor, findBrowserTextScaleFactor()); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 1.25); // = 20px / 16px isCalled = false; root.style.fontSize = '16px'; - await Future.delayed(Duration.zero); + await waitForResizeObserver(); expect(root.style.fontSize, '16px'); expect(isCalled, isTrue); - expect(ui.PlatformDispatcher.instance.textScaleFactor, findBrowserTextScaleFactor()); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 1.0); // = 16px / 16px }); test('calls onMetricsChanged when the typography measurement element size changes', () async { @@ -373,6 +383,65 @@ void testMain() { expect(ui.PlatformDispatcher.instance.paragraphSpacingOverride, expectedParagraphSpacing); }); + // Regression test for https://github.com/flutter/flutter/issues/178856. + test('updates lineHeightScaleFactorOverride only when line-height is explicitly set', () async { + final DomElement root = domDocument.documentElement!; + final DomElement style = createDomHTMLStyleElement(null); + + // Wait for next frame. + Future waitForResizeObserver() { + final completer = Completer(); + domWindow.requestAnimationFrame((_) { + Timer.run(completer.complete); + }); + return completer.future; + } + + addTearDown(() { + style.text = null; + style.remove(); + }); + + style.text = '*{ font-size: 20px !important; }'; + root.append(style); + await waitForResizeObserver(); + expect(root.contains(style), isTrue); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 1.25); // = 20px / 16px + expect(ui.PlatformDispatcher.instance.lineHeightScaleFactorOverride, null); + + style.remove(); + await waitForResizeObserver(); + expect(root.contains(style), isFalse); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 1.0); // = 16px / 16px + expect(ui.PlatformDispatcher.instance.lineHeightScaleFactorOverride, null); + + style.text = '*{ font-size: 20px !important; line-height: 2 !important; }'; + root.append(style); + await waitForResizeObserver(); + expect(root.contains(style), isTrue); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 1.25); // = 20px / 16px + expect(ui.PlatformDispatcher.instance.lineHeightScaleFactorOverride, 2.0); + + style.remove(); + await waitForResizeObserver(); + expect(root.contains(style), isFalse); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 1.0); // = 16px / 16px + expect(ui.PlatformDispatcher.instance.lineHeightScaleFactorOverride, null); + + style.text = '*{ font-size: 32px !important; line-height: 3 !important; }'; + root.append(style); + await waitForResizeObserver(); + expect(root.contains(style), isTrue); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 2.0); // = 32px / 16px + expect(ui.PlatformDispatcher.instance.lineHeightScaleFactorOverride, 3.0); + + style.remove(); + await waitForResizeObserver(); + expect(root.contains(style), isFalse); + expect(ui.PlatformDispatcher.instance.textScaleFactor, 1.0); // = 16px / 16px + expect(ui.PlatformDispatcher.instance.lineHeightScaleFactorOverride, null); + }); + test('disposes all its views', () { final view1 = EngineFlutterView(dispatcher, createDomHTMLDivElement()); final view2 = EngineFlutterView(dispatcher, createDomHTMLDivElement());