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)
Core
Layout
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)
13.48 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → quanxunzhen
Attachment #8586554 -
Flags: review?(dbaron)
This seems like a lot of complexity for something that isn't very important...
Assignee | ||
Comment 3•9 years ago
|
||
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.
Assignee | ||
Comment 6•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ddc17181ea8
https://hg.mozilla.org/mozilla-central/rev/df7c88910f47 https://hg.mozilla.org/mozilla-central/rev/93c2f686e30b
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee | ||
Comment 10•9 years ago
|
||
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 → ---
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 11•9 years ago
|
||
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+
Assignee | ||
Comment 13•9 years ago
|
||
(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.
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ec02fc2f048 (backout) https://hg.mozilla.org/integration/mozilla-inbound/rev/e52a58530232
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e52a58530232
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•