RESOLVED FIXED Bug 150923
REGRESSION(r182286): Tatechuyoko following ruby is drawn too far to the right
https://bugs.webkit.org/show_bug.cgi?id=150923
Summary REGRESSION(r182286): Tatechuyoko following ruby is drawn too far to the right
Myles C. Maxfield
Reported 2015-11-04 20:30:55 PST
Tatechuyoko following ruby is drawn too far to the right
Attachments
Patch (5.04 KB, patch)
2015-11-04 20:36 PST, Myles C. Maxfield
no flags
Patch (9.00 KB, patch)
2015-11-04 21:20 PST, Myles C. Maxfield
zalan: review+
Patch for committing (5.23 KB, patch)
2015-11-05 23:52 PST, Myles C. Maxfield
no flags
Myles C. Maxfield
Comment 1 2015-11-04 20:36:01 PST
Myles C. Maxfield
Comment 2 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).
Myles C. Maxfield
Comment 3 2015-11-04 21:20:43 PST
Myles C. Maxfield
Comment 4 2015-11-05 11:01:05 PST
Myles C. Maxfield
Comment 5 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
Myles C. Maxfield
Comment 6 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.
Myles C. Maxfield
Comment 7 2015-11-05 23:52:32 PST
Created attachment 264920 [details] Patch for committing
Myles C. Maxfield
Comment 8 2015-11-06 15:20:29 PST
Note You need to log in before you can comment on or make changes to this bug.