Last Comment Bug 385866 - [FIXr]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col span="2">
: [FIXr]Crash [@ nsContentUtils::ContentIsDescendantOf] with counters, <col spa...
Status: RESOLVED FIXED
: assertion, crash, testcase, verified1.8.0.14, verified1.8.1.8
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 All
: P1 critical (vote)
: mozilla1.9alpha8
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks: randomclasses stirtable
  Show dependency treegraph
 
Reported: 2007-06-25 21:19 PDT by Jesse Ruderman
Modified: 2011-06-09 14:58 PDT (History)
8 users (show)
jruderman: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (crashes Firefox when loaded) (318 bytes, application/xhtml+xml)
2007-06-25 21:19 PDT, Jesse Ruderman
no flags Details
Fix (1.22 KB, patch)
2007-07-23 23:13 PDT, Boris Zbarsky [:bz]
bernd_mozilla: review+
dbaron: superreview+
dveditz: approval1.8.1.8+
dveditz: approval1.8.0.14+
dbaron: approval1.9+
Details | Diff | Review

Description Jesse Ruderman 2007-06-25 21:19:01 PDT
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 -fullmetaljacket- 2007-06-25 21:56:21 PDT
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
Comment 2 Bernd 2007-06-25 22:04:00 PDT
<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.
Comment 3 Boris Zbarsky [:bz] 2007-07-23 22:55:09 PDT
> 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.
Comment 4 Boris Zbarsky [:bz] 2007-07-23 23:13:31 PDT
Created attachment 273537 [details] [diff] [review]
Fix
Comment 5 Bernd 2007-07-29 06:07:44 PDT
"We shouldn't be setting up quotes/counters for both of them"

Why?

Comment 6 Bernd 2007-07-29 06:55:01 PDT
Comment on attachment 273537 [details] [diff] [review]
Fix

we would enumerate frames rather than content
Comment 7 Boris Zbarsky [:bz] 2007-07-31 10:40:15 PDT
Comment on attachment 273537 [details] [diff] [review]
Fix

We should probably do this on branches too.  This is a _very_ safe fix.
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-07-31 17:19:45 PDT
Comment on attachment 273537 [details] [diff] [review]
Fix

sr=dbaron.  Who knows what's right here -- it probably doesn't really matter.
Comment 9 Boris Zbarsky [:bz] 2007-08-01 22:05:37 PDT
Comment on attachment 273537 [details] [diff] [review]
Fix

Fairly straightforward fix for a crash due to a broken counters list.  Very safe.
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2007-08-02 13:46:02 PDT
Comment on attachment 273537 [details] [diff] [review]
Fix

a19=dbaron
Comment 11 Boris Zbarsky [:bz] 2007-08-02 14:23:56 PDT
Fixed.
Comment 12 Daniel Veditz [:dveditz] 2007-08-29 15:47:16 PDT
Comment on attachment 273537 [details] [diff] [review]
Fix

approved for 1.8.1.7 and 1.8.0.14, a=dveditz for release-drivers
Comment 13 Boris Zbarsky [:bz] 2007-08-30 08:56:31 PDT
Fixed on both branches.
Comment 14 Carsten Book [:Tomcat] 2007-09-03 15:24:44 PDT
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
Comment 15 Stephen Donner [:stephend] - PTO; back on 5/28 2007-12-10 16:57:01 PST
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
Comment 16 Stephen Donner [:stephend] - PTO; back on 5/28 2007-12-10 16:59:21 PST
(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.)
Comment 17 Jesse Ruderman 2007-12-16 22:07:09 PST
Crashtest checked in.

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