370810 - Crash [@ nsHTMLDocument::MatchAnchors] accessing document.anchors for document from removed iframe

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Eli Friedman]

Attachment: Testcase

See testcase.  I think there's just a missing null check in nsHTMLDocument::MatchAnchors, although I'm not sure.

There's also the issue of it and document.links not working, but that's probably not a priority.

Comment 1

[Martijn Wargers (dead)]

Talkback ID: TB29459917E
nsHTMLDocument::MatchAnchors  [mozilla\content\html\document\src\nshtmldocument.cpp, line 1830]
nsContentList::PopulateWithStartingAfter  [mozilla\content\base\src\nscontentlist.cpp, line 761]
nsContentList::PopulateSelf  [mozilla\content\base\src\nscontentlist.cpp, line 810]
nsContentList::BringSelfUpToDate  [mozilla\content\base\src\nscontentlist.cpp, line 852]
XPC_WN_Helper_NewResolve  [mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp, line 1085]
0x038ecf60
0x038ecf40
XPC_WN_NoCall_JSOps

It's also crashing the latest 1.8.1 branch build.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

A simple null check should fix the crash. That could be taken also 
for the branch, I think.

Comment 3

[John J. Barton]

Also triggered by using Firebug 1.1.0b1 w/ Firefox 2.0.0.7 WinXP:
http://code.google.com/p/fbug/issues/detail?id=291

Comment 4

[John J. Barton]

Just in case: Talkback 36050092 and 36049988

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Smaug: You commented, you want to fix it too?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: for trunk

bug 348156 should fix this on trunk, but here is the null check patch anyway.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: for 1.8

For 1.8 the null check is fine, IMO. Similar is used in nsHTMLDocument::MatchLinks. And unless there some other crashes, I wouldn't want
to start clearing all the collection lists. Only mLinks and mAnchors use
nsContentListMatchFunc functions.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #6)
> Created an attachment (id=282237) [details] 
> bug 348156 should fix this on trunk, but here is the null check patch anyway.

Or bug 348156 fixes the bug if ::Destroy stops unbinding elements. Which I think
the latest patch does do.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 282239 [details] [diff] [review]
for 1.8

r/sr=me for the 1.8 branch.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 282239 [details] [diff] [review]
for 1.8

Add a null check to prevent crash. Should be 
a very safe fix for the branch.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 282239 [details] [diff] [review]
for 1.8

approved for 1.8.1.8, a=dveditz for release-drivers

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on branch using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.8) Gecko/20071008 Firefox/2.0.0.8; I don't get a crash with https://bugzilla.mozilla.org/attachment.cgi?id=255573, just an alert with "undefined" as its value.

Replacing fixed1.8.1.8 keyword with verified1.8.1.8

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Assigning to sicking because bug 348156 will fix this on trunk.

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Should be fixed by patch in bug 348156

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

Can't reproduce on trunk Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008020419 Minefield/3.0b3pre, using comment 0.

Verified FIXED.