Bug 236514 - Skip positioned object and line break boxes as they have no affect on width
Summary: Skip positioned object and line break boxes as they have no affect on width
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
Keywords: InRadar
Depends on:
Reported: 2022-02-11 09:53 PST by Brandon
Modified: 2022-02-15 16:01 PST (History)
11 users (show)

See Also:

Patch (2.03 KB, patch)
2022-02-11 09:57 PST, Brandon
no flags Details | Formatted Diff | Diff
Root cause fix (1.91 KB, patch)
2022-02-11 18:20 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (1.93 KB, patch)
2022-02-15 12:35 PST, Brandon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brandon 2022-02-11 09:53:54 PST
Comment 1 Brandon 2022-02-11 09:57:23 PST
Created attachment 451716 [details]
Comment 2 Myles C. Maxfield 2022-02-11 16:10:36 PST
Comment on attachment 451716 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=451716&action=review

> Source/WebCore/rendering/LegacyLineLayout.cpp:571
> +    for (auto* leafChild = rootBox.firstLeafDescendant(); leafChild && i < expansionOpportunities.size(); leafChild = leafChild->nextLeafOnLine()) {

I think this is a band-aid fix. It's not harmful, but it doesn't solve the underlying problem that the invariant is being violated. (The invariant is expansionOpportunities.size() == number of leaves on line.)
Comment 3 Myles C. Maxfield 2022-02-11 17:34:36 PST
The bidi runs are:

1. U+0600
2. U+2E80
3. A ruby run

expansionOpportunities.size() is 2, however.
Comment 4 Myles C. Maxfield 2022-02-11 17:56:15 PST
Whoops, that's wrong.

The first time LegacyLineLayout::computeExpansionForJustifiedText() is called, the runs are:

1. U+0600
2. U+2E80

The second time it's called, the runs are:

1. \n
2. ruby run. Inside this run, the first root box has 2 leaves on the line.
Comment 5 Myles C. Maxfield 2022-02-11 18:00:38 PST
Regarding the second time, both expansion opportunities appended to expansionOpportunities are regarding the insides of the ruby run. For some reason, the previous \n run didn't seem to append anything to expansionOpportunities.
Comment 6 Myles C. Maxfield 2022-02-11 18:20:10 PST
Created attachment 451767 [details]
Root cause fix
Comment 7 Brandon 2022-02-15 12:35:44 PST
Created attachment 452073 [details]
Comment 8 Myles C. Maxfield 2022-02-15 12:38:20 PST
Comment on attachment 452073 [details]

r=me assuming there is a test somewhere
Comment 9 EWS 2022-02-15 16:00:55 PST
Committed r289859 (247300@main): <https://commits.webkit.org/247300@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452073 [details].