Bug 150923 - REGRESSION(r182286): Tatechuyoko following ruby is drawn too far to the right
Summary: REGRESSION(r182286): Tatechuyoko following ruby is drawn too far to the right
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-11-04 20:30 PST by Myles C. Maxfield
Modified: 2015-11-06 15:20 PST (History)
10 users (show)

See Also:


Attachments
Patch (5.04 KB, patch)
2015-11-04 20:36 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch (9.00 KB, patch)
2015-11-04 21:20 PST, Myles C. Maxfield
zalan: review+
Details | Formatted Diff | Diff
Patch for committing (5.23 KB, patch)
2015-11-05 23:52 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2015-11-04 20:30:55 PST
Tatechuyoko following ruby is drawn too far to the right
Comment 1 Myles C. Maxfield 2015-11-04 20:36:01 PST
Created attachment 264843 [details]
Patch
Comment 2 Myles C. Maxfield 2015-11-04 20:37:22 PST
Comment on attachment 264843 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:721
> +        return ForbidLeadingExpansion | ForbidTrailingExpansion;

This actually isn't quite right, for two reasons:

1. There may be a span inside the tatechuyoko
2. If an expansion opportunity is found inside the tatechuyoko, it should be forced to its neighbor (similar to ruby).
Comment 3 Myles C. Maxfield 2015-11-04 21:20:43 PST
Created attachment 264845 [details]
Patch
Comment 4 Myles C. Maxfield 2015-11-05 11:01:05 PST
<rdar://problem/22728382>
Comment 5 Myles C. Maxfield 2015-11-05 11:17:14 PST
Comment on attachment 264845 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:783
> +                    if (FontCascade::leadingExpansionOpportunity(downcast<RenderText>(previousRun->renderer()).stringView(), previousRun->box()->direction())) {

Trailing
Comment 6 Myles C. Maxfield 2015-11-05 22:48:08 PST
Comment on attachment 264845 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:801
> +            ASSERT(textBox.renderer().style().textCombine() == TextCombineHorizontal);
> +            if (nextRun && nextRun->renderer().style().textCombine() == TextCombineNone && textBox.renderer().style().collapseWhiteSpace()) {
> +                ASSERT(!setTrailingExpansion);
> +                setTrailingExpansion = true;
> +                result |= ForbidTrailingExpansion;
> +            }
> +            if (previousRun && previousRun->renderer().style().textCombine() == TextCombineNone && textBox.renderer().style().collapseWhiteSpace()) {
> +                ASSERT(!setLeadingExpansion);
> +                setLeadingExpansion = true;
> +                result |= ForbidLeadingExpansion;
> +            }

Turns out this is all dead code, since tatechuyoko, once combined, is modeled as the Object Replacement Character (U+FFFC), which will never have any expansion opportunities either inside it or directly adjacent to it. The first patch works just as well as this one, and is much simpler.
Comment 7 Myles C. Maxfield 2015-11-05 23:52:32 PST
Created attachment 264920 [details]
Patch for committing
Comment 8 Myles C. Maxfield 2015-11-06 15:20:29 PST
Committed r192120: <http://trac.webkit.org/changeset/192120>