345249 - ASSERTION: inserting after sibling frame with different parent

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

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 (dead)]

Attachment: testcase

Comment 2

[Mats Palmgren (inactive)]

Attachment: 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 (inactive)]

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 (inactive)]

Attachment: Patch rev. 1

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

I buy it.  ;)

Comment 6

[Mats Palmgren (inactive)]

Checked in to trunk at 2006-07-19 21:54 PDT.

-> FIXED

Comment 7

[Martijn Wargers (dead)]

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 (inactive)]

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]]

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]

Cc'ing Robcc and Davel to pickup regression testcase...

Comment 11

[Mats Palmgren (inactive)]

Checked in to MOZILLA_1_8_BRANCH at 2006-07-28 12:05 PDT.

Comment 12

[Dave Liebreich [:davel]]

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]]

Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers

Comment 14

[Mats Palmgren (inactive)]

Checked in to MOZILLA_1_8_0_BRANCH at 2006-08-17 06:41 PDT.