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

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: 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

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Mats Palmgren (inactive)]

Regression window:  2012-12-29 -- 2012-12-30
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0bb4773db082&tochange=eb2f5c66561b

Comment 3

[Mats Palmgren (inactive)]

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

Comment 4

[Mats Palmgren (inactive)]

Attachment: fix+test

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 714811 [details] [diff] [review]
fix+test

r=me

Comment 6

[Scott Johnson (:jwir3)]

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).

Comment 7

[Scott Johnson (:jwir3)]

mats, 

Do you have an idea when this will land?

Comment 8

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab7757dfce20

Comment 9

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/ab7757dfce20

Comment 10

[Scott Johnson (:jwir3)]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Note, this cset should be backed out when landing if approval is granted.
https://hg.mozilla.org/releases/mozilla-aurora/rev/133095cc43fa

Comment 12

[bhavana bajaj [:bajaj]]

Comment on attachment 714811 [details] [diff] [review]
fix+test

Approving for uplift as this will help avoid oranges due to bug 810726

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 714811 [details] [diff] [review]
fix+test

[Approval Request Comment]
Same as Aurora, we're hitting these on Beta too.

Comment 14

[bhavana bajaj [:bajaj]]

Comment on attachment 714811 [details] [diff] [review]
fix+test

Helps fix intermittent orange due to backout of bug 810726

Comment 15

[Scott Johnson (:jwir3)]

Pushed to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/aa3e16175fe9

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/460c676949ce

Comment 17

[Manuela Muntean [Away]]

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

Comment 18

[Tiziana Sellitto [:tiziana]]

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