Closed Bug 1180443 Opened 4 years ago Closed 4 years ago

problem of sizing with ruby in inline-block

Categories

(Core :: Layout: Block and Inline, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox41 --- fixed
firefox42 --- fixed

People

(Reporter: zefling, Assigned: xidorn)

References

Details

(Whiteboard: [mozfr-community])

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150704030210

Steps to reproduce:

In an inline-block, the ruby break at the bad time.

Example : https://jsfiddle.net/t0tzfyz6/
Test render : http://ikilote.net/Galeries/Autres/Divers/Ruby_inline-block.png
Component: Untriaged → Layout: Block and Inline
Product: Firefox → Core
Flags: needinfo?(quanxunzhen)
A simplified testcase:
data:text/html,<div style="display:inline-block"><ruby><rb>a</rb> <rb>b
Flags: needinfo?(quanxunzhen)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 42 Branch → Trunk
This issue is due to the inconsistency of whitespace collapsing between computing intrinsic isize and reflow.
Or this testcase with the whitespace before ruby:
data:text/html,<div style="display:inline-block;"><span>あ</span> <ruby><rb>い</rb><rb>う</rb><rb>え</rb><rt>イ</rt><rt>ウ</rt><rt>エ</rt></ruby></div>
Reduce this a test case:
data:text/html,<div style="display:inline-block"><span>あ</span> <ruby><rb>い<rb>う<rb>え
Duplicate of this bug: 1181766
Attached patch patchSplinter Review
Assignee: nobody → quanxunzhen
Attachment #8631367 - Flags: review?(dbaron)
I don't understand the span bit in nsRubyBaseContainerFrame::AddInlineMinISize.  Why do you copy between aData and data there?
Flags: needinfo?(quanxunzhen)
We hand out the computing to AddInlinePrefISize if there is any span, and in that case, we still need to handle whitespace collapse between this frame and the frame before/after, don't we? If we keep `data` untouched, I don't think AddInlinePrefISize is able to handle that correctly.
Flags: needinfo?(quanxunzhen)
Comment on attachment 8631367 [details] [diff] [review]
patch

r=dbaron if you tested that the reftest fails without the patch and passes with the patch
Attachment #8631367 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/mozilla-central/rev/82c2ba56ae0d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Comment on attachment 8631367 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: CSS ruby enabled in bug 1039006
[User impact if declined]: Have incorrect line wrap if ruby exists in some cases. See url in bug 1181766 for example.
[Describe test coverage new/current, TreeHerder]: Added a new reftest in the proposed patch
[Risks and why]: low risk as CSS ruby has a relatively good test coverage
[String/UUID change made/needed]: n/a
Attachment #8631367 - Flags: approval-mozilla-aurora?
Setting status flags correctly reflect status.
Comment on attachment 8631367 [details] [diff] [review]
patch

Low risk, good test coverage.
Attachment #8631367 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [mozfr-community]
You need to log in before you can comment on or make changes to this bug.