Closed
Bug 457398
Opened 16 years ago
Closed 16 years ago
ASSERTION: First-continuation first-letter should have first-letter style enabled in nsLineLayout
Categories
(Core :: Layout: Block and Inline, defect, P3)
Core
Layout: Block and Inline
Tracking
()
VERIFIED
FIXED
mozilla1.9.1b3
People
(Reporter: bernd_mozilla, Assigned: roc)
References
()
Details
(4 keywords)
Attachments
(3 files, 2 obsolete files)
241 bytes,
text/html
|
Details | |
5.99 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
6.16 KB,
patch
|
Details | Diff | Splinter Review |
the testcase (reftest) from bug 372553 triggers the following assert ###!!! ASSERTION: First-in-flow first-letter should have first-letter style enabled in nsLineLayout!: 'll->GetFirstLetterStyleOK() || GetPrevInFlow()', file d:/moz_src/src/layout/generic/nsFirstLetterFrame.cpp, line 240
Comment 1•16 years ago
|
||
Bug 448610 changed the assertion condition and message, but this testcase still hits it. ###!!! ASSERTION: First-continuation first-letter should have first-letter style enabled in nsLineLayout!: 'll->GetFirstLetterStyleOK() || GetPrevContinuation()', file /Users/jruderman/central/layout/generic/nsFirstLetterFrame.cpp, line 240
Comment 2•16 years ago
|
||
Is it just me, or is the rendering of that testcase incorrect? (I see Bbbbbbbbbbbbbb all blue in both the testcase and the reference)
Comment 3•16 years ago
|
||
I see the incorrect blueness too.
Comment 4•16 years ago
|
||
Relevant part of the frametree: line 0x1610c34: count=2 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8100] {60,60,6552,840} < Inline(span)(1)@0x1610858 next=0x160d270 prev-continuation=0x160cf20 {60,60,6552,840} [state=00000004] [content=0x7258b70] [sc=0x160cde4] < Letter(span)(1)@0x16107a8 next=0x160d238 {0,0,6552,840} [content=0x7258b70] [sc=0x160d090] pst=:first-letter < Text(1)@0x160d484[0,14,T] {0,0,6552,840} [state=80200010] [content=0x7259b80] sc=0x160d118 pst=:-moz-non-element< "Bbbbbbbbbbbbbb" > > Inline(span)(2)@0x160d238 {6552,0,0,840} [content=0x7259bc0] [sc=0x160d008] <> > Text(2)@0x160d270[0,2,T] next=0x160d168 {6612,60,0,840} [state=80400000] [content=0x7259be0] sc=0x160cd94 pst=:-moz-non-element< "\n\n" > > So yes, it's wrong. So is the frame tree. I would assume due to this assertion, since I bet that prevents us creating the overflow stuff for the rest of the text....
Flags: blocking1.9.1?
Comment 5•16 years ago
|
||
And the reason we screw _that_ up is that in nsBlockFrame::DoReflowInlineFrames we have aLineLayout.GetLineNumber() == 1, at least if the frame tree has anything to do with the situation: Block(body)(1)@0x160ce34 {480,480,4020,960} [state=a0100008] [overflow=0,0,6612,960] sc=0x160cb74(i=3,b=0)< line 0x160d300: count=2 state=inline,dirty,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x8021] {60,60,0,0} < Text(0)@0x160cfa4[0,2,T] next=0x160cf20 {60,60,0,0} [state=08600000] [content=0x7258b20] sc=0x160cd94 pst=:-moz-non-element< "\n\n" > Inline(span)(1)@0x160cf20 next=0x1610858 next-continuation=0x1610858 {60,-540,0,840} [state=00000010] [content=0x7258b70] [sc=0x160cde4] < Inline(span)(0)@0x160d058 {0,0,0,840} [content=0x7258b90] [sc=0x160d008] <> > > line 0x1610c34: count=2 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x8100] {60,60,6552,840} < Inline(span)(1)@0x1610858 next=0x160d270 prev-continuation=0x160cf20 {60,60,6552,840} [state=00000004] [content=0x7258b70] [sc=0x160cde4] < Letter(span)(1)@0x16107a8 next=0x160d238 {0,0,6552,840} [content=0x7258b70] [sc=0x160d090] pst=:first-letter < Text(1)@0x160d484[0,14,T] {0,0,6552,840} [state=80200010] [content=0x7259b80] sc=0x160d118 pst=:-moz-non-element< "Bbbbbbbbbbbbbb" > > Inline(span)(2)@0x160d238 {6552,0,0,840} [content=0x7259bc0] [sc=0x160d008] <> > Text(2)@0x160d270[0,2,T] next=0x160d168 {6612,60,0,840} [state=80400000] [content=0x7259be0] sc=0x160cd94 pst=:-moz-non-element< "\n\n" > > line 0x1610890: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x4300] {60,900,0,0} < Text(4)@0x160d168[0,2,T] {60,900,0,0} [state=08600000] [content=0x724f3b0] sc=0x160cd94 pst=:-moz-non-element< "\n\n" > > > Why are we ending up with the first-letter on the second line? In any case, as of July 1 last year this worked correctly in terms of rendering.
Keywords: regression
Comment 6•16 years ago
|
||
Here's the regression range for the rendering: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-10-19+02&maxdate=2007-10-20+02&cvsroot=%2Fcvsroot Looks like a regression from bug 393096.
Blocks: 393096
Comment 7•16 years ago
|
||
This time, I'll check in a reftest that uses <span> to highlight the first letter, dammit. ;)
Comment 8•16 years ago
|
||
Presumably we're breaking on the newlines before that first <span>. Seems to me that breakable whitespace at the immediate start of a block should not be a break opportunity. Certainly breaking on it makes no sense. Similar for end of a block.
Comment 9•16 years ago
|
||
Minimal-ish testcase for the correctness problem: data:text/html,<style>body::first-letter { color: blue }</style><body style="width: 0"> <span></span>Test</body>
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → roc
Assignee | ||
Updated•16 years ago
|
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P3
Assignee | ||
Comment 10•16 years ago
|
||
We don't have to break before "Test" in Boris' testcase, and that's easy to fix. But this testcase is harder; we really do (with our current organization) have to break before "Test" to make sure it gets pushed down below the float.
Assignee | ||
Comment 11•16 years ago
|
||
I think the best thing to do here is simply to not advance the line number when nothing non-empty was placed on the line.
Attachment #348913 -
Flags: superreview?(dbaron)
Attachment #348913 -
Flags: review?(dbaron)
Assignee | ||
Comment 12•16 years ago
|
||
Oops, I forgot to put the test in the last patch.
Attachment #348913 -
Attachment is obsolete: true
Attachment #348916 -
Flags: superreview?(dbaron)
Attachment #348916 -
Flags: review?(dbaron)
Attachment #348913 -
Flags: superreview?(dbaron)
Attachment #348913 -
Flags: review?(dbaron)
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs review]
Comment on attachment 348916 [details] [diff] [review] fix v2 This also looks like it should affect text-indent. After reading section 9.5 of CSS 2.1, the change in terms of text-indent is clearly correct -- in other words, a long first word of a paragraph that can't fit next to a float should be indented when it is pushed below the float. Could you add a test for that as well? r+sr=dbaron with that
Attachment #348916 -
Flags: superreview?(dbaron)
Attachment #348916 -
Flags: superreview+
Attachment #348916 -
Flags: review?(dbaron)
Attachment #348916 -
Flags: review+
Assignee | ||
Comment 14•16 years ago
|
||
With the extra test.
Attachment #348916 -
Attachment is obsolete: true
Attachment #349025 -
Flags: superreview+
Attachment #349025 -
Flags: review+
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs review] → [needs landing]
Comment 16•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/70a0658284a6
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Whiteboard: [needs landing]
Assignee | ||
Updated•16 years ago
|
Flags: in-testsuite+
Updated•16 years ago
|
Keywords: fixed1.9.1
Comment 17•15 years ago
|
||
pushed onto 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/70a0658284a6 also, verified FIXED on debug builds (no assertion): Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre ID:20090608122057 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090608 Minefield/3.6a1pre ID:20090608122028
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•