Closed Bug 370810 Opened 15 years ago Closed 14 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: sharparrow1, Assigned: sicking)

References

Details

(Keywords: testcase, verified1.8.1.8)

Attachments

(3 files)

Attached 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.
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.
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
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+
Attached patch for trunkSplinter Review
bug 348156 should fix this on trunk, but here is the null check patch anyway.
Attached 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+
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
Should be fixed by patch in bug 348156
Status: NEW → RESOLVED
Closed: 14 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.