Closed Bug 1149009 Opened 9 years ago Closed 9 years ago

Ruby intra-level whitespace not contained by any ruby boxes breaks line break suppression

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

When a ruby intra-level whitespace is not contained but only surrounded by ruby boxes, the corresponding style context of the text frames won't have ShouldSuppressLineBreak set. It breaks the assertion that no line break can happen inside ruby content frame.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → quanxunzhen
Attachment #8586554 - Flags: review?(dbaron)
This seems like a lot of complexity for something that isn't very important...
I agree. So probably we should back to bug 1146109?
Comment on attachment 8586554 [details] [diff] [review]
patch

ResolveStyleForNonElement should take a flags enum instead of a boolean,
and then assert that the only flag is is given is eSuppressLineBreak.

>+    eNoFlags = 0,

I'd prefer to just write 0 rather than using an eNoFlags enum.



r=dbaron with that
Attachment #8586554 - Flags: review?(dbaron) → review+
It's probably safer to do this given that the ruby code might get confused by line breaks.
Depends on: 1156169
Given bug 1156169 and 1157666, I think this bug was not completely fixed. In certain condition, the line break won't be correctly suppressed.

I'd like to backout the landed patch, and use a different method to fix it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 1157666, 1156169
No longer depends on: 1156169
Attached patch patchSplinter Review
Attachment #8586554 - Attachment is obsolete: true
Attachment #8600231 - Flags: review?(dbaron)
Comment on attachment 8600231 [details] [diff] [review]
patch

Please add a comment to the nsStyleText::ShouldSuppressLineBreak method warning callers to use the method on nsTextFrame instead.

I don't think the nsBidiPresUtils.cpp change here is needed.

r=dbaron with that
Attachment #8600231 - Flags: review?(dbaron) → review+
(In reply to David Baron [:dbaron] ✈ from comment #12)
> Comment on attachment 8600231 [details] [diff] [review]
> patch
> 
> Please add a comment to the nsStyleText::ShouldSuppressLineBreak method
> warning callers to use the method on nsTextFrame instead.
> 
> I don't think the nsBidiPresUtils.cpp change here is needed.

It is needed, or there will be a compile error.
https://hg.mozilla.org/mozilla-central/rev/e52a58530232
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.