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

RESOLVED FIXED in mozilla1.9alpha8

Status

()

Core
Layout: Tables
P1
critical
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: bz)

Tracking

(Blocks: 2 bugs, 5 keywords)

Trunk
mozilla1.9alpha8
x86
All
assertion, crash, testcase, verified1.8.0.14, verified1.8.1.8
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
Created attachment 269795 [details]
testcase (crashes Firefox when loaded)

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.

Comment 1

10 years ago
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

Comment 2

10 years ago
<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.
Created attachment 273537 [details] [diff] [review]
Fix
Assignee: nobody → bzbarsky
Attachment #269795 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #273537 - Flags: superreview?(dbaron)
Attachment #273537 - Flags: review?(bernd_mozilla)
(Assignee)

Updated

10 years ago
Priority: -- → P1
Summary: Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2"> → [FIX]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">
Target Milestone: --- → mozilla1.9beta1

Comment 5

10 years ago
"We shouldn't be setting up quotes/counters for both of them"

Why?

Comment 6

10 years ago
Comment on attachment 273537 [details] [diff] [review]
Fix

we would enumerate frames rather than content
Attachment #273537 - Flags: review?(bernd_mozilla) → review+
(Assignee)

Updated

10 years ago
Summary: [FIX]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2"> → [FIXr]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">
(Assignee)

Updated

10 years ago
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+
(Assignee)

Updated

10 years ago
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
Last Resolved: 10 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.
Keywords: fixed1.8.0.14, fixed1.8.1.7
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
Keywords: fixed1.8.1.7 → verified1.8.1.7
(Reporter)

Updated

10 years ago
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
Keywords: fixed1.8.0.14 → 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.)
(Reporter)

Comment 17

10 years ago
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.