Last Comment Bug 345249 - ASSERTION: inserting after sibling frame with different parent
: ASSERTION: inserting after sibling frame with different parent
Status: RESOLVED FIXED
: assertion, fixed1.8.0.7, fixed1.8.1, testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Mats Palmgren (:mats)
:
Mentors:
Depends on:
Blocks: 342758
  Show dependency treegraph
 
Reported: 2006-07-19 15:07 PDT by Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
Modified: 2006-11-10 12:15 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (183 bytes, text/html)
2006-07-19 15:08 PDT, Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( )
no flags Details
stack (6.06 KB, text/html)
2006-07-19 18:16 PDT, Mats Palmgren (:mats)
no flags Details
Patch rev. 1 (1.41 KB, patch)
2006-07-19 18:33 PDT, Mats Palmgren (:mats)
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.0.7+
mbeltzner: approval1.8.1+
Details | Diff | Review

Description Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-07-19 15:07:02 PDT
See upcoming testcase, in which I get this assertion, after that I get another assertion, which might be bug 342758.

###!!! ASSERTION: inserting after sibling frame with different parent: '!aPrevFr
ame || aPrevFrame->GetParent() == this', file c:/mozilla/mozilla/layout/generic/
nsBlockFrame.cpp, line 5396
###!!! ASSERTION: prev sibling not in line list: 'Not Reached', file c:/mozilla/
mozilla/layout/generic/nsBlockFrame.cpp, line 5472
Comment 1 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-07-19 15:08:11 PDT
Created attachment 229897 [details]
testcase
Comment 2 Mats Palmgren (:mats) 2006-07-19 18:16:48 PDT
Created attachment 229910 [details]
stack

We're inserting a new legend frame into a fieldset that already has
a legend and thus we reparent it and put it in the content area frame.
In this particular case we have |aPrevFrame == mLegendFrame| and since we
reparented the inserted frame |aPrevFrame| becomes invalid (we should use
nsnull instead). (This is similar to the issues we had with <caption>s
recently.)
Comment 3 Mats Palmgren (:mats) 2006-07-19 18:19:05 PDT
It wouldn't surprise me if this can lead to crashes during frame destruction
later on...  I have a patch coming up... (it fixes bug 342758 too)
Comment 4 Mats Palmgren (:mats) 2006-07-19 18:33:10 PDT
Created attachment 229912 [details] [diff] [review]
Patch rev. 1
Comment 5 Boris Zbarsky [:bz] 2006-07-19 20:35:59 PDT
Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

I buy it.  ;)
Comment 6 Mats Palmgren (:mats) 2006-07-19 22:42:53 PDT
Checked in to trunk at 2006-07-19 21:54 PDT.

-> FIXED
Comment 7 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2006-07-22 09:51:07 PDT
Mats, is the patch something for 1.8.1 branch? I don't get the assertion from this bug, but I get still the assertion from bug 342758 on branch.
I know I made the testcase from a (not minimised) testcase that once crashed my trunk build, but it was a one-time only crash.
Comment 8 Mats Palmgren (:mats) 2006-07-25 15:45:43 PDT
Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

(In reply to comment #7)
Yeah, as I said in comment 3 it wouldn't surprise me if this could
lead to crashes caused by use of freed memory.
The fix is low risk.
Comment 9 Mike Beltzner [:beltzner, not reading bugmail] 2006-07-26 10:54:00 PDT
Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

a=drivers. Please land this on the MOZILLA_1_8_BRANCH.
Comment 10 Mike Schroepfer 2006-07-26 10:55:24 PDT
Cc'ing Robcc and Davel to pickup regression testcase...
Comment 11 Mats Palmgren (:mats) 2006-07-28 12:47:30 PDT
Checked in to MOZILLA_1_8_BRANCH at 2006-07-28 12:05 PDT.
Comment 12 Dave Liebreich [:davel] 2006-08-11 10:19:06 PDT
re: regression testcase.

How would a script running in content (or chrome) detect the pass/fail condition for this issue?

Or should the test just load the page and look for the assert (or better yet, force the app to crash on assert.  Does XPCOM_DEBUG_BREAK work on non-debug builds?)?
Comment 13 Daniel Veditz [:dveditz] 2006-08-15 15:49:31 PDT
Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Comment 14 Mats Palmgren (:mats) 2006-08-17 08:27:57 PDT
Checked in to MOZILLA_1_8_0_BRANCH at 2006-08-17 06:41 PDT.

Note You need to log in before you can comment on or make changes to this bug.