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
###!!! 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
Attachment #713205 - Attachment mime type: text/html → text/html; charset=utf-8
Regression window: 2012-12-29 -- 2012-12-30 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0bb4773db082&tochange=eb2f5c66561b
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): https://bugzilla.mozilla.org/attachment.cgi?id=601726&action=diff#a/layout/generic/nsBlockFrame.cpp_sec18
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) https://tbpl.mozilla.org/?tree=Try&rev=f132761991b9
Attachment #714811 - Flags: review?(bzbarsky)
Comment on attachment 714811 [details] [diff] [review] fix+test r=me
Attachment #714811 - Flags: review?(bzbarsky) → review+
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).
mats, Do you have an idea when this will land?
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment on attachment 714811 [details] [diff] [review] fix+test 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. https://hg.mozilla.org/releases/mozilla-aurora/rev/133095cc43fa
Comment on attachment 714811 [details] [diff] [review] fix+test 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] fix+test [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] fix+test Helps fix intermittent orange due to backout of bug 810726
Attachment #714811 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/aa3e16175fe9
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
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.