ASSERTION: inserting after sibling frame with different parent

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: mats)

Tracking

(4 keywords)

Trunk
assertion, fixed1.8.0.7, fixed1.8.1, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
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
(Reporter)

Comment 1

11 years ago
Created attachment 229897 [details]
testcase
(Assignee)

Comment 2

11 years ago
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.)
(Assignee)

Comment 3

11 years ago
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)
Assignee: nobody → mats.palmgren
Blocks: 342758
OS: Windows XP → All
Hardware: PC → All
(Assignee)

Comment 4

11 years ago
Created attachment 229912 [details] [diff] [review]
Patch rev. 1
Attachment #229912 - Flags: superreview?(bzbarsky)
Attachment #229912 - Flags: review?(bzbarsky)
Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

I buy it.  ;)
Attachment #229912 - Flags: superreview?(bzbarsky)
Attachment #229912 - Flags: superreview+
Attachment #229912 - Flags: review?(bzbarsky)
Attachment #229912 - Flags: review+
(Assignee)

Comment 6

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

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

11 years ago
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.
(Assignee)

Comment 8

11 years ago
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.
Attachment #229912 - Flags: approval1.8.1?
Attachment #229912 - Flags: approval1.8.0.6?
Comment on attachment 229912 [details] [diff] [review]
Patch rev. 1

a=drivers. Please land this on the MOZILLA_1_8_BRANCH.
Attachment #229912 - Flags: approval1.8.1? → approval1.8.1+

Comment 10

11 years ago
Cc'ing Robcc and Davel to pickup regression testcase...
(Assignee)

Comment 11

11 years ago
Checked in to MOZILLA_1_8_BRANCH at 2006-07-28 12:05 PDT.
Keywords: fixed1.8.1
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 on attachment 229912 [details] [diff] [review]
Patch rev. 1

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #229912 - Flags: approval1.8.0.7? → approval1.8.0.7+
(Assignee)

Comment 14

11 years ago
Checked in to MOZILLA_1_8_0_BRANCH at 2006-08-17 06:41 PDT.
Keywords: fixed1.8.0.7
You need to log in before you can comment on or make changes to this bug.