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

VERIFIED FIXED

Status

()

defect
P1
critical
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: sharparrow1, Assigned: sicking)

Tracking

({testcase, verified1.8.1.8})

Trunk
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

12 years ago
Posted file 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.

Updated

12 years ago
Depends on: 348156
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.
Summary: Crash accessing document.anchors for document from removed iframe → Crash [@ nsHTMLDocument::MatchAnchors] accessing document.anchors for document from removed iframe
A simple null check should fix the crash. That could be taken also 
for the branch, I think.

Comment 3

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

12 years ago
Just in case: Talkback 36050092 and 36049988
Flags: blocking1.9?
Smaug: You commented, you want to fix it too?
Assignee: general → Olli.Pettay
Flags: blocking1.9? → blocking1.9+
Posted patch for trunkSplinter Review
bug 348156 should fix this on trunk, but here is the null check patch anyway.
Posted patch for 1.8Splinter Review
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.
Attachment #282239 - Flags: superreview?(jonas)
Attachment #282239 - Flags: review?(jonas)
(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 on attachment 282239 [details] [diff] [review]
for 1.8

r/sr=me for the 1.8 branch.
Attachment #282239 - Flags: superreview?(jonas)
Attachment #282239 - Flags: superreview+
Attachment #282239 - Flags: review?(jonas)
Attachment #282239 - Flags: review+
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.
Attachment #282239 - Flags: approval1.8.1.8?
Comment on attachment 282239 [details] [diff] [review]
for 1.8

approved for 1.8.1.8, a=dveditz for release-drivers
Attachment #282239 - Flags: approval1.8.1.8? → approval1.8.1.8+

Updated

12 years ago
Keywords: fixed1.8.1.8
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
Assigning to sicking because bug 348156 will fix this on trunk.
Assignee: Olli.Pettay → jonas
Priority: -- → P1
Should be fixed by patch in bug 348156
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.