crash nsXMLPrettyPrinter::Unhook , I believe document.replaceChild (in bookmarklet) crashes FF 7.x

RESOLVED FIXED in mozilla10

Status

()

Core
DOM
P1
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: J Del Monte, Assigned: bz)

Tracking

({crash, regression})

Trunk
mozilla10
x86
Windows 7
crash, regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 564094 [details]
FF_fail.txt

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0.1) Gecko/20100101 Firefox/7.0.1
Build ID: 20110928134238

Steps to reproduce:

Tried using a bookmarklet that worked fine in FF 6.x


Actual results:

FF crashes (tried in both W2K and 64 bit Win 7)


Expected results:

Should have worked - returns a link from within the XML document.   I think it fails at: document.replaceChild(f,document.firstChild)

Comment 1

6 years ago
Please provide a crash id or stacktrace -> https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report

If you could reduce the test case to the bare minimum that would also help
Keywords: crash
(Reporter)

Comment 2

6 years ago
(In reply to Tim (fmdeveloper) from comment #1)
> Please provide a crash id or stacktrace ->
> https://developer.mozilla.org/en/How_to_get_a_stacktrace_for_a_bug_report
> 
> If you could reduce the test case to the bare minimum that would also help

Hopefully this is what you are looking for:
http://crash-stats.mozilla.com/report/index/bp-e5f85efa-050b-402e-ba2b-513b82111002

By test case do you mean the XML document or the bookmarklet?   The XML sample is what the portable ding returns so it is a true test case.   I didn't write the bookmarlket from heck so I cannot really trim it, sorry.

Comment 3

6 years ago
bp-30247a53-b238-421d-b562-680b12111002


Regression Window(cached m-c hourly),
Works:
http://hg.mozilla.org/mozilla-central/rev/5c6d107ede5a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110601 Firefox/7.0a1 ID:20110601030746
Crashes:
http://hg.mozilla.org/mozilla-central/rev/840644b2b6a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110601 Firefox/7.0a1 ID:20110601011530
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c6d107ede5a&tochange=840644b2b6a2

Triggered by:
Bug 657353 - Stop sending BeginUpdate/EndUpdate on content state changes
Blocks: 657353
Severity: normal → critical
Status: UNCONFIRMED → NEW
Crash Signature: [@ nsXMLPrettyPrinter::Unhook()]
Component: General → DOM
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
QA Contact: general → general
Hardware: x86_64 → x86
Summary: I believe document.replaceChild (in bookmarklet) crashes FF 7.x → crash nsXMLPrettyPrinter::Unhook , I believe document.replaceChild (in bookmarklet) crashes FF 7.x

Comment 4

6 years ago
Created attachment 564100 [details]
xml

Comment 5

6 years ago
[str]
1.open attachment 564100 [details]
2.the following bookmarklet should be bookmarked,
3.open the bookmarklet.

[actual]
browser crashes

javascript:(function(){function%20j(b,a,c){typeof%20c==="undefined"&&(c=document);typeof%20a==="undefined"&&(a=0);b=c.getElementsByTagName(b);return%20b.length<a+1?"":b[a].firstChild.data}function%20h(b,a,c){return{attrName:b,attrVal:a,text:c}}function%20g(b){var%20a=document.createElementNS("http://www.w3.org/1999/xhtml",b),c;arguments.length>1&&(c=arguments[1],c.attrName.length&&c.attrVal.length&&a.setAttribute(c.attrName,c.attrVal),c.text.length&&a.appendChild(document.createTextNode(c.text)));return%20a}function%20i(b){for(var%20a=%20b.length-1;a>0;a--)b[a-1].appendChild(b[a])}(function(){for(var%20b=void%200,a=void%200,c=void%200,b=b?b:a?a.ownerDocument:document,f=[],a=b.evaluate("//headline",a?a:b,c?c:null,XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,null),b=0;b<a.snapshotLength;b++)f[b]=a.snapshotItem(b);var%20d,e,k,l;if(f.length===0)alert("No%20messages!");else{e=window.location.toString().replace("getMessages","createSession");window.XMLHttpRequest?(xhttp=new%20XMLHttpRequest,xhttp.open("GET",e,!1),xhttp.send(""),e=xhttp.responseXML):(alert("Sorry,%20your%20browswer%20can't%20handle%20this!"),%20e=!1);j("status",0,e)!=="0"?alert("Unable%20to%20create%20session;%20verify%20installId%20and%20cksum"):d=j("disc",0,e);b="&disc="+d;e=g("ul",h("style","margin:2em;",""),"");for(d=0;d<f.length;d++)a=j("activationText",0,f[d]).replace(/(<([^>]+)>)/ig,""),c=j("activationUrl",0,f[d]),c.length>0?(j("type",0,f[d])==="offer"?(c+=b,k=j("activationNewDelta",0,f[d])*1,l=g("span",h("style","margin-left:2em;%20font-size:80%;","Active%20in%20"+(k/60).toFixed(0)+":"+((k%%2060)/100).toFixed(2)*100))):k=0,i([e,g("li"),g("a",h("href",c,%20a))]),k>0&&i([e.lastChild,l])):i([e,g("li",h("","",a))])}f=g("div");i([f,e]);l=g("html:div",h("style","margin:%202.5em%205em;%20color:%20red;%20font-size:%2090%;%20font-style:%20italic;",""));e=g("p",h("style","padding:%205px;",""));d=g("span",h("style","border:%201px%20solid%20black;%20padding:2px;","Caution:%20You%20are%20using%20Beta%20v0.1%20of%20this%20script.%20Be%20sure%20to%20"));d.appendChild(g("a",h("href","http://www.flyertalk.com/forum/16382603-post8.html","check%20periodically")));d.appendChild(document.createTextNode("%20for%20an%20updated%20version."));%20i([l,e,d]);i([f,l]);i([f,g("p",h("style","font-size:80%;%20margin:2em;",document.lastModified))]);document.replaceChild(f,document.firstChild)})()})();

Comment 6

6 years ago
Sorry the above bookmarklet fails to run due to cut and paste problem or something.

Pls use bookmarklet described in attachment 564094 [details]

Updated

6 years ago
So mDocument is null in nsXMLPrettyPrinter::Unhook.

What happens is that the replaceChild call triggers a remove and an insert which both call MaybeUnhook.  So we get two runnables to unhook posted, and then the second one of those to run loses.

And the reason we get two runnables posted is because the check is like so:

    if (!aContent || !aContent->GetBindingParent() && !mUnhookPending) {

which is clearly bogus; it tests true any time !aContent, which is the case here because aContent is the aContainer from ContentInserted/ContentRemoved, which is null for replacement of the document element.
Assignee: nobody → bzbarsky
Priority: -- → P1
Whiteboard: [need review]
Version: 7 Branch → Trunk
Created attachment 564109 [details] [diff] [review]
Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.
Attachment #564109 - Flags: review?(jonas)
Thank you all for the clear steps to reproduce!
Comment on attachment 564109 [details] [diff] [review]
Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.

Thanks!
Attachment #564109 - Flags: review?(jonas) → review+
Whiteboard: [need review] → [need landing]
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a26e1aa86e5
Flags: in-testsuite+
Whiteboard: [need landing]
Target Milestone: --- → mozilla10
Comment on attachment 564109 [details] [diff] [review]
Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.

It may be worth taking this on aurora.  This is a very safe change that fixes a null-deref crash that web content can actually trigger pretty easily (see the mochitest).

On the other hand, I expect our frequency for this crash is pretty low...
Attachment #564109 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/5a26e1aa86e5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 14

6 years ago
Comment on attachment 564109 [details] [diff] [review]
Fix stupid logic error in the XML prettyprinter that can cause a null-dereference crash.

Crash level is very, very low. We'll let this come up in the normal process.
Attachment #564109 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Duplicate of this bug: 714232
You need to log in before you can comment on or make changes to this bug.