Closed Bug 385866 Opened 14 years ago Closed 13 years ago

[FIXr]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">

Categories

(Core :: Layout: Tables, defect, P1)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: jruderman, Assigned: bzbarsky)

References

(Blocks 2 open bugs)

Details

(5 keywords)

Crash Data

Attachments

(2 files)

Loading the testcase (in a Mac trunk debug build of Firefox) causes three assertions and a crash:

###!!! ASSERTION: identical: 'pseudoType1 != pseudoType2', 
file nsGenConList.cpp, line 124

###!!! ASSERTION: null check on startContent should be sufficient to null check nodeContent as well, since if nodeContent is for the root, startContent (which is before it) must be too: 'nodeContent || !startContent', 
file nsCounterManager.cpp, line 145

###!!! ASSERTION: The possible descendant is null!: 'aPossibleDescendant', 
file nsContentUtils.cpp, line 1144

Crash (null dereference) with stack trace:
  0  nsINode::GetNodeParent
  1  nsContentUtils::ContentIsDescendantOf
  2  nsCounterList::SetScope
  ...

Bug 383129 has a similar crash signature, but the patch there is a patch to xul tree code, so this isn't a dup.
testcase also crashed winxp sp2

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070625 Minefield/3.0a6pre ID:2007062515
OS: Mac OS X → All
<col id="col" span="2"></col> creates two col frames which share the style context and the content node the first colframe is content based and the second is anonymous col due to the span.

I guess that ASSERTION: identical: 'pseudoType1 != pseudoType2', 
file nsGenConList.cpp, line 124 is bogus in this case.

The next assert is probably the result of keeping a reference to the second anonymous col frame past the removal of the <col> node.
> creates two col frames which share the style context and the content node

We shouldn't be setting up quotes/counters for both of them.  That's the real bug here.
Attached patch FixSplinter Review
Assignee: nobody → bzbarsky
Attachment #269795 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #273537 - Flags: superreview?(dbaron)
Attachment #273537 - Flags: review?(bernd_mozilla)
Priority: -- → P1
Summary: Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2"> → [FIX]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">
Target Milestone: --- → mozilla1.9beta1
"We shouldn't be setting up quotes/counters for both of them"

Why?

Comment on attachment 273537 [details] [diff] [review]
Fix

we would enumerate frames rather than content
Attachment #273537 - Flags: review?(bernd_mozilla) → review+
Summary: [FIX]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2"> → [FIXr]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">
Summary: [FIXr]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2"> → [FIX]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">
Comment on attachment 273537 [details] [diff] [review]
Fix

We should probably do this on branches too.  This is a _very_ safe fix.
Attachment #273537 - Flags: approval1.8.1.7?
Attachment #273537 - Flags: approval1.8.0.13?
Comment on attachment 273537 [details] [diff] [review]
Fix

sr=dbaron.  Who knows what's right here -- it probably doesn't really matter.
Attachment #273537 - Flags: superreview?(dbaron) → superreview+
Summary: [FIX]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2"> → [FIXr]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">
Comment on attachment 273537 [details] [diff] [review]
Fix

Fairly straightforward fix for a crash due to a broken counters list.  Very safe.
Attachment #273537 - Flags: approval1.9?
Comment on attachment 273537 [details] [diff] [review]
Fix

a19=dbaron
Attachment #273537 - Flags: approval1.9? → approval1.9+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Attachment #273537 - Flags: approval1.8.0.13? → approval1.8.0.14?
Comment on attachment 273537 [details] [diff] [review]
Fix

approved for 1.8.1.7 and 1.8.0.14, a=dveditz for release-drivers
Attachment #273537 - Flags: approval1.8.1.7?
Attachment #273537 - Flags: approval1.8.1.7+
Attachment #273537 - Flags: approval1.8.0.14?
Attachment #273537 - Flags: approval1.8.0.14+
Fixed on both branches.
verified fixed 1.8.1.7 using the testcase and Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.7pre) Gecko/2007090308 BonEcho/2.0.0.7pre + Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.7pre) Gecko/20070903 BonEcho/2.0.0.7pre ID:2007090304

no crash on testcase with this builds - adding verified keyword
Attachment #269795 - Attachment is obsolete: false
No crash here with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre and Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre, using the testcase in comment 0.

Replacing fixed1.8.0.14 with verified1.8.0.14
(Second build ID should be Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre; copy/paste is at times non-functional in a VM, for some reason.)
Crashtest checked in.
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsContentUtils::ContentIsDescendantOf]
You need to log in before you can comment on or make changes to this bug.