Skip to content

Conversation

@ksokolovskyi
Copy link
Contributor

Fixes #178856
Fixes #178271
Fixes #178238

Description

  • Fixes an issue in lineHeightScaleFactorOverride calculation, causing it to have abnormal values.
  • Integrates text scale factor update into recently added _addTypographySettingsObserver. The ResizeObserver on the typography probe element is getting notified each time the font size changes, because it leads to the element's size change.
  • Removes DomMutationObserver previously used for font size observations.
  • Adds/Updates tests to verify the fixes.
BEFORE AFTER
before.mov
after.mov

Demo

https://flutter-text-scale-factor.web.app

Code sample
import 'package:flutter/material.dart';

void main() {
  runApp(const App());
}

class App extends StatelessWidget {
  const App({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      debugShowCheckedModeBanner: false,
      home: Screen(),
    );
  }
}

class Screen extends StatefulWidget {
  const Screen({super.key});

  @override
  State<Screen> createState() => _ScreenState();
}

class _ScreenState extends State<Screen> with SingleTickerProviderStateMixin {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Padding(
          padding: const EdgeInsets.all(8),
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            spacing: 10,
            children: [
              Builder(
                builder: (context) {
                  print(
                    'lineHeightScaleFactorOverride: ${MediaQuery.maybeLineHeightScaleFactorOverrideOf(context)}',
                  );
                  final scale = MediaQuery.textScalerOf(context).scale(1);

                  return Text(
                    'Scale ${scale.toStringAsFixed(3)}',
                    style: TextStyle(
                      fontSize: 30,
                      backgroundColor: Colors.grey.shade200,
                    ),
                    textAlign: TextAlign.center,
                  );
                },
              ),
            ],
          ),
        ),
      ),
    );
  }
}

Pre-launch Checklist

@github-actions github-actions bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Nov 20, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses an issue where onTextScaleFactorChanged was not being called correctly. It refactors the text scale factor observation to use a ResizeObserver instead of a DomMutationObserver, which is a more robust solution. The changes also include a fix for lineHeightScaleFactorOverride calculation and updated tests. I've identified a critical bug in the new _updateTextScaleFactor implementation that could lead to incorrect behavior, and a minor code duplication issue in the tests. With these fixes, the PR looks solid.

Comment on lines 389 to 396
// Wait for next frame.
Future<void> waitForResizeObserver() {
final Completer<void> completer = Completer<void>();
domWindow.requestAnimationFrame((_) {
Timer.run(completer.complete);
});
return completer.future;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The helper function waitForResizeObserver is duplicated. It was already defined earlier in the same test group (lines 284-291). To improve maintainability and reduce code duplication, this function should be defined only once at a scope accessible to both tests that use it, for example, at the beginning of the group.

@ksokolovskyi
Copy link
Contributor Author

Hi @Renzo-Olivares, I've added you as a reviewer as you are the author of the recently added typography settings observer on the web, and this PR updates lineHeightScaleFactorOverride calculation logic, which fixes the issue described here #178856

Could you please take a look at this PR when you have time?

Copy link
Contributor

@Renzo-Olivares Renzo-Olivares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ksokolovskyi thank you for the fixes and clean up! I left a few comments but mostly looks good to me.

@ksokolovskyi
Copy link
Contributor Author

@Renzo-Olivares, thanks a lot for your review! I applied your suggestions in the latest commit. Would you mind taking a look again?

Copy link
Contributor

@Renzo-Olivares Renzo-Olivares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, modulo any feedback from @mdebbar. Thank you for the contribution!

@ksokolovskyi
Copy link
Contributor Author

@Renzo-Olivares thanks a lot for your review!

Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you for the fixes!

@ksokolovskyi
Copy link
Contributor Author

@mdebbar thanks a lot for review!

@ksokolovskyi ksokolovskyi added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 2, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Dec 2, 2025
Merged via the queue into flutter:master with commit daddf7a Dec 2, 2025
180 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 3, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Dec 3, 2025
flutter/flutter@5545bb3...e274574

2025-12-03 [email protected] Roll Skia from 20829e37dfb8 to db4c79d41513 (1 revision) (flutter/flutter#179401)
2025-12-03 [email protected] Roll Skia from adc7ea94cada to 20829e37dfb8 (6 revisions) (flutter/flutter#179385)
2025-12-03 [email protected] Refactor GetShaderClipDepth for clarity (flutter/flutter#179110)
2025-12-03 [email protected] Roll Skia from 3b339a83959b to adc7ea94cada (1 revision) (flutter/flutter#179376)
2025-12-03 [email protected] Roll Dart SDK from eb743a1d4ade to 0bb365d7ac74 (7 revisions) (flutter/flutter#179372)
2025-12-03 [email protected] feat: Add `mainAxisExtent` parameter to `GridView` constructors (flutter/flutter#176927)
2025-12-03 [email protected] Roll Skia from eb01fff20df8 to 3b339a83959b (4 revisions) (flutter/flutter#179371)
2025-12-02 [email protected] Fix crash when text editing value changes between scrolls (flutter/flutter#179163)
2025-12-02 [email protected] Roll Skia from 6bd3b06b1e08 to eb01fff20df8 (3 revisions) (flutter/flutter#179362)
2025-12-02 [email protected] Adds Impellerc flatbuffer format versioning. (flutter/flutter#175470)
2025-12-02 [email protected] Adds format argument to Picture.toImageSync (flutter/flutter#178691)
2025-12-02 [email protected] Delete disabled workflow and add missing permissions key to workflow (flutter/flutter#178911)
2025-12-02 [email protected] [web] Fix some gn warnings (flutter/flutter#178313)
2025-12-02 [email protected] Roll Skia from 45337c4e919d to 6bd3b06b1e08 (4 revisions) (flutter/flutter#179353)
2025-12-02 [email protected] [ios] Reland Dynamic Content Resizing (flutter/flutter#179153)
2025-12-02 [email protected] [web] Fix onTextScaleFactorChanged not getting called. (flutter/flutter#178862)
2025-12-02 [email protected] Roll Packages from c8be05d to 148dcd2 (9 revisions) (flutter/flutter#179343)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
Fixes flutter#178856
Fixes flutter#178271
Fixes flutter#178238

### Description

- Fixes an issue in `lineHeightScaleFactorOverride` calculation, causing
it to have abnormal values.
- Integrates text scale factor update into recently added
`_addTypographySettingsObserver`. The `ResizeObserver` on the typography
probe element is getting notified each time the font size changes,
because it leads to the element's size change.
- Removes `DomMutationObserver` previously used for font size
observations.
- Adds/Updates tests to verify the fixes.

| BEFORE | AFTER |
| - | - |
| <video
src="https://github.com/user-attachments/assets/f62fb3c6-63e1-447f-b4ef-8f22ad944a0a"/>
| <video
src="https://github.com/user-attachments/assets/c220dd30-89d9-4f76-a43f-b908a940fafb"/>
|

### Demo

https://flutter-text-scale-factor.web.app

<details closed><summary>Code sample</summary>

```dart
import 'package:flutter/material.dart';

void main() {
  runApp(const App());
}

class App extends StatelessWidget {
  const App({super.key});

  @OverRide
  Widget build(BuildContext context) {
    return const MaterialApp(
      debugShowCheckedModeBanner: false,
      home: Screen(),
    );
  }
}

class Screen extends StatefulWidget {
  const Screen({super.key});

  @OverRide
  State<Screen> createState() => _ScreenState();
}

class _ScreenState extends State<Screen> with SingleTickerProviderStateMixin {
  @OverRide
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: Padding(
          padding: const EdgeInsets.all(8),
          child: Column(
            mainAxisAlignment: MainAxisAlignment.center,
            spacing: 10,
            children: [
              Builder(
                builder: (context) {
                  print(
                    'lineHeightScaleFactorOverride: ${MediaQuery.maybeLineHeightScaleFactorOverrideOf(context)}',
                  );
                  final scale = MediaQuery.textScalerOf(context).scale(1);

                  return Text(
                    'Scale ${scale.toStringAsFixed(3)}',
                    style: TextStyle(
                      fontSize: 30,
                      backgroundColor: Colors.grey.shade200,
                    ),
                    textAlign: TextAlign.center,
                  );
                },
              ),
            ],
          ),
        ),
      ),
    );
  }
}
```

</details>

## Pre-launch Checklist

- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [X] I signed the [CLA].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [X] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [X] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels. platform-web Web applications specifically

Projects

None yet

3 participants