Skip to content

Assert TextStyle height is not NaN#186617

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
mvincentong:assert-textstyle-height-not-nan
Jul 10, 2026
Merged

Assert TextStyle height is not NaN#186617
auto-submit[bot] merged 1 commit into
flutter:masterfrom
mvincentong:assert-textstyle-height-not-nan

Conversation

@mvincentong

Copy link
Copy Markdown
Contributor

Fixes #175248

Prevents TextStyle(height: double.nan) and getParagraphStyle(height: double.nan) from passing NaN line heights down to dart:ui in debug mode. This produces a clear assertion before platform-specific text layout failures.

Tests:

  • ./bin/dart format --output=none --set-exit-if-changed packages/flutter/lib/src/painting/text_style.dart packages/flutter/test/painting/text_style_test.dart
  • ./bin/flutter test packages/flutter/test/painting/text_style_test.dart
  • ./bin/flutter analyze packages/flutter/lib/src/painting/text_style.dart packages/flutter/test/painting/text_style_test.dart
  • git diff --check

@github-actions github-actions Bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels May 16, 2026

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

Copy link
Copy Markdown
Contributor

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 adds assertions to the TextStyle constructor and getParagraphStyle method to ensure the height parameter is not NaN, and includes unit tests to verify this behavior. Feedback recommends defining the error message as a private static constant to avoid duplication and improve maintainability, adhering to the repository's style guide.

Comment thread packages/flutter/lib/src/painting/text_style.dart Outdated
Comment thread packages/flutter/lib/src/painting/text_style.dart Outdated
@mvincentong
mvincentong force-pushed the assert-textstyle-height-not-nan branch from 53bf3d1 to b9307bf Compare May 16, 2026 10:41
@mvincentong
mvincentong changed the base branch from main to master May 21, 2026 03:01
justinmc
justinmc previously approved these changes May 21, 2026

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

Comment thread packages/flutter/lib/src/painting/text_style.dart Outdated
Comment thread packages/flutter/lib/src/painting/text_style.dart
@justinmc justinmc added the CICD Run CI/CD label May 21, 2026
@mvincentong
mvincentong force-pushed the assert-textstyle-height-not-nan branch from b9307bf to f9923ae Compare May 22, 2026 08:21
@github-actions github-actions Bot removed the CICD Run CI/CD label May 22, 2026
@mvincentong

Copy link
Copy Markdown
Contributor Author

Addressed the review nit in f9923ae by removing the redundant String type from the private const. Re-ran formatter, full text_style_test.dart, targeted analyze, and git diff --check.

@Renzo-Olivares Renzo-Olivares left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thank you for the contribution!

@Renzo-Olivares
Renzo-Olivares requested a review from justinmc June 18, 2026 20:14
@mvincentong

Copy link
Copy Markdown
Contributor Author

This is approved and all visible checks are green, but it does not have the CICD label/full presubmit run. I do not have label permissions; could a maintainer add CICD when convenient?

@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 25, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 25, 2026
@auto-submit

auto-submit Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

auto label is removed for flutter/flutter/186617, Failed to enqueue flutter/flutter/186617 with HTTP 400: Pull request Required status check "Merge Queue Guard" is expected..

@mvincentong

Copy link
Copy Markdown
Contributor Author

Looks like the autosubmit attempt hit Required status check "Merge Queue Guard" is expected after the checks had completed. This still has two approvals and the visible checks are green from here; could a maintainer re-apply the landing labels when convenient?

@Renzo-Olivares Renzo-Olivares added the CICD Run CI/CD label Jul 9, 2026
@Renzo-Olivares
Renzo-Olivares force-pushed the assert-textstyle-height-not-nan branch from a416a28 to bbbd5a4 Compare July 9, 2026 18:36
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Jul 9, 2026
@Renzo-Olivares Renzo-Olivares added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Jul 9, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Jul 10, 2026
Merged via the queue into flutter:master with commit bc6a4c8 Jul 10, 2026
98 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 10, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jul 10, 2026
flutter/flutter@dc2a870...f7b66f3

2026-07-10 [email protected] [iOS][test] Fix VSyncClient display link deallocation test on iOS 27 (flutter/flutter#188627)
2026-07-10 [email protected] Fix broken listener code in WindowScope (flutter/flutter#189208)
2026-07-10 [email protected] [web] Preserve text field focus across tab switches (flutter/flutter#188738)
2026-07-10 [email protected] [Impeller] Recycle HostBuffer arena entries only after GPU completion (flutter/flutter#188965)
2026-07-10 [email protected] Assert TextStyle height is not NaN (flutter/flutter#186617)
2026-07-10 [email protected] Roll Fuchsia Test Scripts from dFkTCiDxEtPxYK5Nn... to wLST_A-xfOeGT_5mj... (flutter/flutter#189259)
2026-07-10 [email protected] Roll Dart SDK from a11fb7ed40a5 to 0fc1668c4af4 (5 revisions) (flutter/flutter#189257)
2026-07-10 [email protected] Adds a missing `await` to a `FutureOr` (flutter/flutter#189198)
2026-07-10 [email protected] [web] Repair RenderCanvas CSS size drift (flutter/flutter#188797)
2026-07-10 [email protected] Roll Fuchsia Test Scripts from s5_gZFJ8De9AJalTw... to dFkTCiDxEtPxYK5Nn... (flutter/flutter#189215)
2026-07-10 [email protected] Roll Fuchsia Linux SDK from QcRFUtvCw2EobfJ8s... to czpzDg9ABY2oKLAOY... (flutter/flutter#189222)
2026-07-10 [email protected] Update Depdencies Used By `flutter_engine_group_performance` (flutter/flutter#189229)

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],[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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems CICD Run CI/CD framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Set TextStyle(height: double.nan), App Crash

5 participants