Closed Bug 840818 Opened 7 years ago Closed 7 years ago

"ASSERTION: value should always be stored and non-empty when state set" with nested -moz-column, odd characters


(Core :: Layout, defect)

Not set



Tracking Status
firefox20 --- verified
firefox21 --- fixed
firefox22 --- verified


(Reporter: jruderman, Assigned: mats)


(Blocks 2 open bugs)


(Keywords: assertion, regression, testcase)


(3 files)

Attached file testcase
###!!! ASSERTION: value should always be stored and non-empty when state set: 'prop && !prop->mLines.empty() && prop->mLines.front()->GetChildCount() == 0 ? prop->mFrames.IsEmpty() : prop->mLines.front()->mFirstChild == prop->mFrames.FirstChild()', file layout/generic/nsBlockFrame.cpp, line 4580
Attached file stack
Attachment #713205 - Attachment mime type: text/html → text/html; charset=utf-8
Assignee: nobody → matspal
The regression range above isn't the real problem, it's just that bug 822053
fixed an error that prevented the testcase to reach the problematic code.
The real "regression" is bug 728908 that added the additional conditions
to the assertion in RemoveOverflowLines (line 4589):
Attached patch fix+testSplinter Review
The good news is that the code is working correctly, it's just the assert
conditions are stricter than it was before.

This patch cleans up nsBlockFrame::PullFrameFrom so that it avoids the
problem.  Instead of RemoveOverflowLines() followed by SetOverflowLines()
if there are still lines remaining, I'm doing DestroyOverflowLines()
just in case the last line was removed.

(I'll clean up PullFrame/PullFrameFrom a bit more (making it faster)
in a follow-up bug after I land the other nsFrameList bugs)
Attachment #714811 - Flags: review?(bzbarsky)
Comment on attachment 714811 [details] [diff] [review]

Attachment #714811 - Flags: review?(bzbarsky) → review+
Blocks: 843332
Blocks: 810726
The assertions described show up in the reftest for column-balancing-overflow-004 when the patch for bug 810726 is applied, so this kind of blocks the landing of that bug until it lands. (The patch on this bug fixes that assertion, though).

Do you have an idea when this will land?
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment on attachment 714811 [details] [diff] [review]

We need this on aurora for bug 810726, since otherwise we will have oranges for the test in bug 810726.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: We'll have assertions in test cases on aurora in the test for bug 810726.
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Minimal, I believe. 
String or UUID changes made by this patch: None
Attachment #714811 - Flags: approval-mozilla-aurora?
Note, this cset should be backed out when landing if approval is granted.
Comment on attachment 714811 [details] [diff] [review]

Approving for uplift as this will help avoid oranges due to bug 810726
Attachment #714811 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 714811 [details] [diff] [review]

[Approval Request Comment]
Same as Aurora, we're hitting these on Beta too.
Attachment #714811 - Flags: approval-mozilla-beta?
Comment on attachment 714811 [details] [diff] [review]

Helps fix intermittent orange due to backout of bug 810726
Attachment #714811 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified fixed with the latest beta debug build, on a Mac OSX 10.7.5 machine. The assertion doesn't show anymore.

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:20.0) Gecko/20130313 Firefox/20.0
Build ID: 20130313174412
QA Contact: manuela.muntean
Verified fixed with the latest aurora debug build, on a Mac OSX 10.8.3

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20130512 Firefox/22.0
Build ID: 20130512150817
You need to log in before you can comment on or make changes to this bug.