Skip to content

[web_paragraph] Map code units without a cluster to a cluster - #195384

Merged
auto-submit[bot] merged 4 commits into
flutter:masterfrom
dbebawy:webparagraph-clusterless-code-units
Nov 2, 2026
Merged

[web_paragraph] Map code units without a cluster to a cluster#195384
auto-submit[bot] merged 4 commits into
flutter:masterfrom
dbebawy:webparagraph-clusterless-code-units

Conversation

@dbebawy

@dbebawy dbebawy commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Chrome gives some invisible characters, such as a soft hyphen (U+00AD), a zero width space (U+200B) or a right-to-left mark (U+200F), no text cluster. WebParagraph left their code units mapped to cluster 0, so a text range that includes one can map to the wrong clusters. On master:

  • Laying out a paragraph that ends with a soft hyphen or zero width space fails the assertion in _TextClusterMapping.toTextRange.
  • Laying out a paragraph whose only text is one of these characters throws a StateError.
  • Wrapping text where a space is followed by one of these characters, such as 'ab \u200F\u200Fcd', fails an assertion in TextWrapper.breakLines.
  • getLineNumberAt fails an assertion for a line that starts with one of these characters.
  • A right-to-left mark at the start of an LTR paragraph makes the first letter count twice in the line width.
  • A styled span that contains only one of these characters adds an empty box to getBoxesForRange.

This maps each of those code units to the cluster before it, or to the first cluster at the start of the text, so a range that starts or ends at one of them does not take a cluster next to it. Bidi regions and span slices with no clusters add no bidi run and no block to the line, and a line's text range starts where the previous line ended and ends where the next line starts. A paragraph with no clusters is laid out like empty text, and the line breaker also finds a soft line break that lands on one of these characters.

This was the first commit of #195366 (soft hyphens in WebParagraph, for #193506). @Rusino found three of these cases while reviewing that PR, wrote the fixes and tests in #195377, and suggested landing this part on its own first. Her two commits from #195386 add the rest: paragraphs with only these characters, and line breaks next to them.

Testing

paragraph_edge_cases_test.dart has a new group for these characters. Eleven of its tests fail on master. The two tests for a soft line break before a direction mark after a zero width space pass on master with Chrome 145 on my machine, so they may depend on the Chrome version.

I ran the chrome-dart2js-webparagraph-ui suite locally on macOS with felt test --gcs-prod. Everything passes except "Paragraph with different locales for the same language" and "paint overflows" in paragraph_test.dart, which fail the same way on master on my machine.

Pre-launch Checklist

@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically team-web Owned by Web platform team labels Oct 27, 2026
@dbebawy dbebawy mentioned this pull request Oct 27, 2026
Open
10 of 11 tasks
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Oct 27, 2026
View reviewed changes

@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 updates the web text layout engine to properly handle clusterless characters, such as soft hyphens, zero-width spaces, and directional marks, by mapping them to adjacent clusters and preventing empty visual runs or disconnected line ranges. Corresponding edge-case tests are also added. The review feedback identifies a potential RangeError in _TextClusterMapping.toClusterRange when _clusters is empty (e.g., for paragraphs containing only clusterless characters) and suggests adding a guard clause along with a test case to cover this scenario.

Comment thread engine/src/flutter/lib/web_ui/lib/src/engine/web_paragraph/layout.dart Resolved
Comment thread engine/src/flutter/lib/web_ui/test/webparagraph/paragraph_edge_cases_test.dart Resolved
@Rusino Rusino mentioned this pull request Oct 27, 2026
Closed
@Rusino
Rusino self-requested a review October 27, 2026 15:06
Rusino
Rusino previously approved these changes Oct 29, 2026
View reviewed changes
@Rusino
Rusino requested a review from mdebbar October 29, 2026 14:29
dbebawy and others added 3 commits October 30, 2026 09:45
@dbebawy @Rusino
[web_paragraph] Map code units without a cluster to a cluster
d6c9ee0 
Chrome gives some invisible characters, such as a soft hyphen (U+00AD)
or a zero width space (U+200B), no text cluster. Their code units were
left mapped to cluster 0, so a text range that included one could map
to a cluster range that went backwards. Laying out a paragraph that
ends with one of these characters failed the assertion in
_TextClusterMapping.toTextRange.

Map each of those code units to the cluster before it, or to the first
cluster at the start of the text. A cluster range that starts at such a
code unit then starts at the next cluster, so a span that starts with
one covers only its own clusters. A cluster range that ends at one at
the start of the text ends before the first cluster.

Bidi regions and span slices that contain only these characters have
no clusters, so they add no bidi run and no block to the line. A line's
text range starts where the previous line ended, or at 0, so a line
that starts with one of these characters still contains it.

Co-authored-by: Julia Lavrova <[email protected]>
@Rusino @dbebawy
[web_paragraph] Add failing clusterless tests
03e2628 
Add contract tests for paragraphs containing only clusterless characters,
soft line breaks on clusterless code units before directional marks, and
clusterless characters following trailing spaces. Without the fix, all 6
new tests fail (3 StateError crashes on lines.first, 2 line count
mismatches 1 vs 2, and 1 AssertionError at wrapper.dart:101).

Assisted-by: Gemini Pro Next
@Rusino @dbebawy
[web_paragraph] Fix clusterless layout edge cases
f50fa68 
Check allClusters.length == 1 in TextLayout.wrapText and lines.isEmpty in
TextLayout._calculatePaintBounds so non-empty paragraphs containing only
clusterless characters (such as U+00AD, U+200B, or U+200F) get empty-text
metrics and Rect.zero paintBounds instead of crashing on lines.first.

Scan codeUnitFlags from the previous cluster's end to the current cluster's
start in TextWrapper._isSoftLineBreak so soft line breaks that land on
unmapped clusterless code units before directional marks are not missed, and
use hardlineTextRange.start in TextLayout.addLine so clusterless characters
after trailing spaces are included in allTextRange. Without the fix, the 6
new tests in paragraph_edge_cases_test.dart fail (3 StateError crashes on
lines.first, 2 line count mismatches 1 vs 2, and 1 AssertionError at
wrapper.dart:101).

Assisted-by: Gemini Pro Next
@dbebawy
dbebawy force-pushed the webparagraph-clusterless-code-units branch from 5512fe0 to f50fa68 Compare October 30, 2026 13:51
@dbebawy

dbebawy commented Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Thank you for the review. I rebased onto master after #194980. The only conflict was the comment in extractBidiRuns, and the webparagraph tests pass locally. The push dismissed your approval, so could you take another look and add the CICD label when you have a chance?

mdebbar
mdebbar previously approved these changes Oct 30, 2026
View reviewed changes

@mdebbar mdebbar 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.

Thanks for the fix!

Comment thread engine/src/flutter/lib/web_ui/test/webparagraph/paragraph_edge_cases_test.dart Outdated
Comment on lines +956 to +958
// A soft break opportunity after U+200B (LB8) lands on the immediately following
// directional mark, which has no cluster of its own. Line wrapping must still detect the
// soft break before the next visible cluster.

@mdebbar mdebbar Oct 30, 2026

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.

On Chrome 145 these pass without the fix, so the break doesn't always land on the mark:

Suggested change
// A soft break opportunity after U+200B (LB8) lands on the immediately following
// directional mark, which has no cluster of its own. Line wrapping must still detect the
// soft break before the next visible cluster.
// Depending on the browser's line breaker, the soft break after U+200B (LB8) can land on
// the following directional mark, which has no cluster of its own. Line wrapping must
// still detect the soft break before the next visible cluster.

@dbebawy dbebawy Oct 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you, that is clearer. Applied in 4cf9b62.

Comment thread engine/src/flutter/lib/web_ui/lib/src/engine/web_paragraph/layout.dart Outdated
Comment on lines +287 to +288
// Use hardlineTextRange.start rather than whitespaceTextRange.end so unmapped clusterless
// characters following trailing spaces are included in this line's text range.

@mdebbar mdebbar Oct 30, 2026

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.

These characters are mapped now (to the cluster before them):

Suggested change
// Use hardlineTextRange.start rather than whitespaceTextRange.end so unmapped clusterless
// characters following trailing spaces are included in this line's text range.
// Use hardlineTextRange.start rather than whitespaceTextRange.end so clusterless characters
// following trailing spaces are included in this line's text range.

@dbebawy dbebawy Oct 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you, that is clearer. Applied in 4cf9b62.

Comment thread engine/src/flutter/lib/web_ui/lib/src/engine/web_paragraph/layout.dart Resolved
Rusino
Rusino previously approved these changes Oct 30, 2026
View reviewed changes
@Rusino Rusino added the CICD Run CI/CD label Oct 30, 2026
@dbebawy
dbebawy dismissed stale reviews from Rusino and mdebbar via 4cf9b62 October 31, 2026 15:05
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Oct 31, 2026
@dbebawy

dbebawy commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

I applied both suggestions from @mdebbar, which only change comments. The push dismissed the approvals and the CICD label, so could one of you re-approve and add CICD and autosubmit when you have a chance?

@Rusino
Rusino requested review from Rusino and mdebbar November 1, 2026 00:40
@Rusino Rusino added the CICD Run CI/CD label Nov 1, 2026
@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@auto-submit

auto-submit Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

auto label is removed for flutter/flutter/195384, Failed to enqueue flutter/flutter/195384 with HTTP 400: Something went wrong while executing your query on 2026-11-02T16:09:08Z. Please include EE0E:1CCAC6:275F28E:8130295:6AE8B5A2 when reporting this issue..

@dbebawy

dbebawy commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@mdebbar @Rusino Thank you both for the re-approvals! It looks like the auto-submit bot hit a GitHub error (HTTP 400) while enqueuing and removed the autosubmit label. Could you re-add it when you have a chance?

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Nov 2, 2026
Merged via the queue into flutter:master with commit 31a0908 Nov 2, 2026
36 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2026
@dbebawy
dbebawy deleted the webparagraph-clusterless-code-units branch November 2, 2026 19:00
gmackall pushed a commit to gmackall/flutter that referenced this pull request Nov 3, 2026
@dbebawy @Rusino
[web_paragraph] Map code units without a cluster to a cluster (flutte…
8fb275a 
…r#195384)

Chrome gives some invisible characters, such as a soft hyphen (U+00AD),
a zero width space (U+200B) or a right-to-left mark (U+200F), no text
cluster. WebParagraph left their code units mapped to cluster 0, so a
text range that includes one can map to the wrong clusters. On master:

* Laying out a paragraph that ends with a soft hyphen or zero width
space fails the assertion in `_TextClusterMapping.toTextRange`.
* Laying out a paragraph whose only text is one of these characters
throws a `StateError`.
* Wrapping text where a space is followed by one of these characters,
such as `'ab \u200F\u200Fcd'`, fails an assertion in
`TextWrapper.breakLines`.
* `getLineNumberAt` fails an assertion for a line that starts with one
of these characters.
* A right-to-left mark at the start of an LTR paragraph makes the first
letter count twice in the line width.
* A styled span that contains only one of these characters adds an empty
box to `getBoxesForRange`.

This maps each of those code units to the cluster before it, or to the
first cluster at the start of the text, so a range that starts or ends
at one of them does not take a cluster next to it. Bidi regions and span
slices with no clusters add no bidi run and no block to the line, and a
line's text range starts where the previous line ended and ends where
the next line starts. A paragraph with no clusters is laid out like
empty text, and the line breaker also finds a soft line break that lands
on one of these characters.

This was the first commit of flutter#195366 (soft hyphens in WebParagraph, for
flutter#193506). @Rusino found three of these cases while reviewing that PR,
wrote the fixes and tests in flutter#195377, and suggested landing this part on
its own first. Her two commits from flutter#195386 add the rest: paragraphs
with only these characters, and line breaks next to them.

## Testing

`paragraph_edge_cases_test.dart` has a new group for these characters.
Eleven of its tests fail on master. The two tests for a soft line break
before a direction mark after a zero width space pass on master with
Chrome 145 on my machine, so they may depend on the Chrome version.

I ran the `chrome-dart2js-webparagraph-ui` suite locally on macOS with
`felt test --gcs-prod`. Everything passes except "Paragraph with
different locales for the same language" and "paint overflows" in
`paragraph_test.dart`, which fail the same way on master on my machine.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [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].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [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.
- [ ] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[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/website]: https://github.com/flutter/website
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Julia Lavrova <[email protected]>
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 3, 2026
Merged
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Nov 3, 2026
@engine-flutter-autoroll
Roll Flutter from fcfb4f203eae to 934720261be0 (38 revisions) (#13540)
9da3462 
Roll Flutter from fcfb4f203eae to 934720261be0 (38 revisions)

flutter/flutter@fcfb4f2...9347202

2026-11-03 [email protected] Roll Skia from 5bceaa033999 to c733ad3e37cb (5 revisions) (flutter/flutter#195872)
2026-11-03 [email protected] Roll Skia from 34d2bf479385 to 5bceaa033999 (9 revisions) (flutter/flutter#195864)
2026-11-03 [email protected] fix: prefer pointer events over semantics tap for mouse clicks on web (flutter/flutter#191529)
2026-11-03 [email protected] [VPAT] Expose a semantics text field value while it is not being edited (flutter/flutter#195445)
2026-11-03 [email protected] [Impeller] Make a GL context current before constructing an AiksContext without a surface (flutter/flutter#195629)
2026-11-03 [email protected] [flutter_tools] Eliminate globals.dart from project.dart (flutter/flutter#194066)
2026-11-03 [email protected] [tool] Complete context runner elimination and migrate AppContext usages to ToolContext (flutter/flutter#190797)
2026-11-03 [email protected] [web] Unify font collections between CanvasKit and Skwasm (flutter/flutter#195539)
2026-11-03 [email protected] [Android] Replay predictive back state for retained engines (flutter/flutter#192286)
2026-11-03 [email protected] [vulkan]: Adds depth/stencil synchronization for subpasses (flutter/flutter#195689)
2026-11-03 [email protected] [web] Honor requested upscaling in instantiateImageCodecWithSize and ImageDescriptor.instantiateCodec (flutter/flutter#195836)
2026-11-03 [email protected] Roll Skia from 866b61ca92c9 to 34d2bf479385 (9 revisions) (flutter/flutter#195848)
2026-11-03 [email protected] Roll Fuchsia Test Scripts from xl1LWBuppzu-BnOzV... to B6zarykZp6PKWRueg... (flutter/flutter#195847)
2026-11-03 [email protected] Remove unused ffi (flutter/flutter#195710)
2026-11-02 [email protected] Revert: [Android] Let the raster thread use every core of a chip without little cores (flutter/flutter#195838)
2026-11-02 [email protected] Run 10 impeller iOS benchmarks as one devicelab task (flutter/flutter#195661)
2026-11-02 [email protected] Use named uniforms instead of setFloat in StretchEffect (flutter/flutter#195372)
2026-11-02 [email protected] [flutter_tools] Report when the browser disconnects during a web test run (flutter/flutter#192198)
2026-11-02 [email protected] fix: Resolve memory leak in image_list example (flutter/flutter#189526)
2026-11-02 [email protected] Only run the common local engine configurations in presubmit (flutter/flutter#195688)
2026-11-02 [email protected] Update gcloud_version to version [email protected] (flutter/flutter#195769)
2026-11-02 [email protected] [Impeller] Order the Vulkan copy-texture-to-texture layout transition after the copy (flutter/flutter#195814)
2026-11-02 [email protected] [Impeller] Order the Vulkan ResizeTexture layout transition after its blit (flutter/flutter#195721)
2026-11-02 [email protected] [web] Fix CanvasKit fragment shader uniforms being freed or mutated after recording (flutter/flutter#195718)
2026-11-02 [email protected] Roll Fuchsia Linux SDK from MqypI6RNyVZ_FLUE-... to T_-6-6X1lDMghhAvX... (flutter/flutter#195809)
2026-11-02 [email protected] [Android] Log why Impeller falls back to OpenGLES (flutter/flutter#194764)
2026-11-02 [email protected] Roll Skia from 3f35fb6e591b to 866b61ca92c9 (11 revisions) (flutter/flutter#195802)
2026-11-02 [email protected] Roll Skia from 3f35fb6e591b to 866b61ca92c9 (11 revisions) (flutter/flutter#195802)
2026-11-02 [email protected] Only run the common local engine configurations in presubmit (flutter/flutter#195688)
2026-11-02 [email protected] Balance tool_integration_tests subshards by expected duration (flutter/flutter#195628)
2026-11-02 [email protected] [web_paragraph] Map code units without a cluster to a cluster (flutter/flutter#195384)
2026-11-02 [email protected] [web] Hide synthetic autofill inputs from assistive technology (flutter/flutter#195618)
2026-11-02 [email protected] Roll Packages from 3742a4e to 3f34a6f (8 revisions) (flutter/flutter#195795)
2026-11-02 [email protected] [web] Unskip web tests that pass on CanvasKit and Skwasm: platform_view_test, route_notification_messages_test (flutter/flutter#195605)
2026-11-02 [email protected] Roll Skia from e5ad3868b8db to 3f35fb6e591b (1 revision) (flutter/flutter#195789)
2026-11-02 [email protected] Roll Skia from f5215e74aa16 to e5ad3868b8db (3 revisions) (flutter/flutter#195779)
2026-11-02 [email protected] Roll Skia from e43478a2f9f8 to f5215e74aa16 (1 revision) (flutter/flutter#195776)
2026-11-01 [email protected] Roll Fuchsia Linux SDK from kW0A0xxNkqf38ljm4... to MqypI6RNyVZ_FLUE-... (flutter/flutter#195768)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode 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
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdebbar mdebbar mdebbar approved these changes
@Rusino Rusino Rusino approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-web Web applications specifically team-web Owned by Web platform team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dbebawy @mdebbar @Rusino