313400 - document.createRange().extractContents() crashes [@ nsRange::CloneContents]

Status: VERIFIED FIXED

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

Description

[shutdown]

document.createRange().extractContents(); crashes.
TB10965825H

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051021 Firefox/1.6a1

nsRange::CloneContents  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsRange.cpp, line 1736]
nsRange::ExtractContents  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsRange.cpp, line 1658]
XPCWrappedNative::CallMethod  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2139]
XPC_WN_CallMethod  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1444]
js_Invoke  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1177]
js_Interpret  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3501]
js_Execute  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1408]
JS_EvaluateUCScriptForPrincipals  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 4118]
nsJSContext::EvaluateString  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1072]
nsJSThunk::EvaluateScript  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp, line 297]

Comment 1

[Gérard Talbot]

Upon clicking the URL, I crashed with Seamonkey 1.1a rv:1.9a1 build 2005102208
but I did not crash with Firefox 1.5 rv:1.8b5 build 20051021; XP Pro SP2 here.
Though Firefox 1.5 javascript console reports
"
Error: uncaught exception: [Exception... "Component returned failure code:
0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIDOMRange.extractContents]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: javascript:
document.createRange().extractContents(); :: <TOP_LEVEL> :: line 1"  data: no]
"

CONFIRMING

Comment 2

[Gérard Talbot]

I couldn't find a duplicate for this bug.

Adding clean-report and talkbackid keywords

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Attachment: full stack trace

Comment 4

[Alex Vincent [:WeirdAl]]

Related:
###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../dist/include/xpcom\nsCOMPtr.h, line 849

Comment 5

[Alex Vincent [:WeirdAl]]

Oh ho.  The mStartParent is the HTML document.  Documents have no owner document, so we get null back at line 1725:

res = mStartParent->GetOwnerDocument(getter_AddRefs(document));

From there, we go downhill.

Patch coming up shortly.

Comment 6

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1

After this patch, we get NS_ERROR_FAILURE.  The range is empty.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 200882 [details] [diff] [review]
patch, v1

+  nsCOMPtr<nsIDOMDocument> document(do_QueryInterface(mStartParent));
+  if (!document) {
+    res = mStartParent->GetOwnerDocument(getter_AddRefs(document));
+    if (NS_FAILED(res)) return res;
+  }

The common case here is that mStartParent is *not* a document, so this would be faster over all if you flipped this around (since you wouldn't be trying to QI to nsIDOMDocument when you know it will fail in most cases), i.e.:

+  nsCOMPtr<nsIDOMDocument> document;
+  res = mStartParent->GetOwnerDocument(getter_AddRefs(document));
+  if (NS_FAILED(res)) return res;
+  if (!document) {
+    document = do_QueryInterface(mStartParent);
+  }

r+sr=jst either way though.

Comment 8

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1.1

Per #developers, I've also added an assertion for document, and for optimized builds, we return if we don't have a document.  I'm carrying forward jst's r+sr.  I'd appreciate someone checking this in for me, please.

Comment 9

[Alex Vincent [:WeirdAl]]

jst has checked in my patch, thank you.

Comment 10

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

Verified FIXED using javascript: document.createRange().extractContents(); in the URL bar with build 2005-10-27-04 on Windows XP SeaMonkey trunk.

Comment 11

[Martijn Wargers (dead)]

There are quite a few crashes on the 1.8.1 branch, with this stacktrace:
http://talkback-public.mozilla.org/search/start.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsRange::CloneContents&vendor=MozillaOrg&product=Firefox2&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid
I can reproduce it with the description in http://talkback-public.mozilla.org/search/start.jsp?search=2&type=iid&id=23034895
Demo of typo3 here:
http://www.typo3board.net/demo/typo3/alt_main.php

Comment 12

[Martijn Wargers (dead)]

Comment on attachment 200886 [details] [diff] [review]
patch, v1.1

It also crashes on the 1.8.0.x branch, using the typo3 demo.

Comment 13

[Mike Schroepfer]

Comment on attachment 200886 [details] [diff] [review]
patch, v1.1

Differing to 1.8.1.1

Comment 14

[Adam Guthrie]

*** Bug 353473 has been marked as a duplicate of this bug. ***

Comment 15

[Adam Guthrie]

*** Bug 354829 has been marked as a duplicate of this bug. ***

Comment 16

[Martijn Wargers (dead)]

Comment on attachment 200886 [details] [diff] [review]
patch, v1.1

Maybe this can be reconsidered?
It is numer 30 on the firefox2 branch crash list, so it's not really a
topcrasher, but the patch is almost a year in trunk and seems very
safe to me.

Comment 17

[Mike Schroepfer]

Comment on attachment 200886 [details] [diff] [review]
patch, v1.1

Still want to wait for this till 1.8.1.1 - thanks for bringing this back up!

Comment 18

[Kasper L. Nielsen]

The Typo3 community discovered the problem immediately after 1.5.0.7 came out. Most are admins and rolled back to 1.5.0.6. The actual number of people affected are larger but it is masked by the rollbacks.

If this does not make it into 1.5.0.8 and 2.0 I hope for an immediate update very soon after.

Comment 19

[Martijn Wargers (dead)]

Oh great, the typo3 demo doesn't crash with Firefox1.5.0.6, but does crash after I've updated to Firefox1.5.07 :(

Comment 20

[Mike Schroepfer]

Comment on attachment 200886 [details] [diff] [review]
patch, v1.1

Ok.  Given we have some folks who are known to be affected let's get this in for RC2.

Comment 21

[Alex Vincent [:WeirdAl]]

The patch applies cleanly to Gecko 1.8 branch with an offset of -23 lines.  Need checkin.

Comment 22

[Alex Vincent [:WeirdAl]]

Also applies cleanly to Gecko 1.8.0 branch, offset -31 lines.

Comment 23

[Martijn Wargers (dead)]

Checking in nsRange.cpp;
/cvsroot/mozilla/content/base/src/nsRange.cpp,v  <--  nsRange.cpp
new revision: 1.189.2.6; previous revision: 1.189.2.5
done

Checked into the 1.8.1 branch.
I just verified in my debug 1.8.1 branch build that it indeed fixes the typo3 crash.

Comment 24

[Martijn Wargers (dead)]

Comment on attachment 200886 [details] [diff] [review]
patch, v1.1

This was checked in on the 1.8.1 branch, so it will also need to be checked in on the 1.8.0.8 branch.

Comment 25

[Adam Guthrie]

*** Bug 354995 has been marked as a duplicate of this bug. ***

Comment 26

[Martijn Wargers (dead)]

*** Bug 355011 has been marked as a duplicate of this bug. ***

Comment 27

[Daniel Veditz [:dveditz]]

Comment on attachment 200886 [details] [diff] [review]
patch, v1.1

approved for 1.8.0 branch, a=dveditz for drivers

Comment 28

[Martijn Wargers (dead)]

Checking in nsRange.cpp;
/cvsroot/mozilla/content/base/src/nsRange.cpp,v  <--  nsRange.cpp
new revision: 1.189.6.2; previous revision: 1.189.6.1
done

Checked into the 1.8.0.x branch.

Comment 29

[Jay Patel [:jay]]

v.fixed on 1.8.0 branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.8pre) Gecko/20061020 Firefox/1.5.0.8pre