| Summary: | Inline ruby does not get justified correctly | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||||||
| Component: | New Bugs | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | buildbot, commit-queue, dino, esprehn+autocc, glenn, hyatt, jonlee, koivisto, kondapallykalyan, rniwa, simon.fraser, thorton, webkit-bug-importer, zalan | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 144044 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Myles C. Maxfield
2014-10-04 01:01:38 PDT
Created attachment 239272 [details]
Patch
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 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
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 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
Created attachment 239280 [details]
Patch
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. Created attachment 239503 [details]
Patch
Comment on attachment 239503 [details]
Patch
r=me
|