Bug 137421 – Inline ruby does not get justified correctly

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Myles C. Maxfield]

Inline ruby does not get justified correctly

[Myles C. Maxfield]

Created attachment 239272 [details]
Patch

[Build Bot]

Comment on attachment 239272 [details]
Patch

Attachment 239272 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5689605747638272

New failing tests:
fast/ruby/bopomofo-rl.html

[Build Bot]

Created attachment 239274 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 239272 [details]
Patch

Attachment 239272 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5105455835643904

New failing tests:
fast/ruby/bopomofo-rl.html

[Build Bot]

Created attachment 239275 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Myles C. Maxfield]

Created attachment 239280 [details]
Patch

[Dave Hyatt]

Comment on attachment 239280 [details]
Patch

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

r-

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:582
> +        if (auto* rubyText = rubyRun.rubyText())
> +            rubyText->setLogicalLeft(rubyText->logicalLeft() + totalExpansion / 2);

Everything looks fine except for this part. You can't just move the ruby text and assume that's all that was needed. Ruby text could change its size and position based off the additional width gained by the ruby base. A few test cases to consider that would illustrate problems with this approach: (a) text-align: right on the ruby text, (b) justified ruby text, (c) ruby text wider than the ruby base.

Here is what I would do instead. In this function:

(1) Compute the total expansion of the ruby base without changing any widths. You already do this.
(2) Set the OVERRIDE WIDTH of the ruby run to its old width plus the totalExpansion. This will cause it to lay out and size using this modified width.
(3) Relayout the ruby run. It will justify the text properly because it will use the override width and increase its size. The line layout will then re-run and use the expanded space properly. The ruby text will be repositioned by ruby layout and just do the right thing with the expanded width.

[Myles C. Maxfield]

Created attachment 239503 [details]
Patch

[Dave Hyatt]

Comment on attachment 239503 [details]
Patch

r=me

[Myles C. Maxfield]

http://trac.webkit.org/changeset/174489

[Myles C. Maxfield]

<rdar://problem/17352909>