As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 489050 - [FIX]Strange rendering bug when javascript is executed: everything is rendered twice
: [FIX]Strange rendering bug when javascript is executed: everything is rendere...
: fixed1.9.1, regression, verified1.9.0.12
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: x86 Windows Vista
: P2 normal (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
: Jet Villegas (:jet)
Depends on: 396226
Blocks: 392318 490187
  Show dependency treegraph
Reported: 2009-04-19 00:08 PDT by Yongqian
Modified: 2009-07-21 17:13 PDT (History)
19 users (show)
dbaron: blocking1.9.1+
dveditz: blocking1.9.0.12+
dveditz: wanted1.9.0.x+
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

xpi + html to reproduce bug (36.21 KB, application/zip)
2009-04-19 00:09 PDT, Yongqian
no flags Details
Proposed fix (7.03 KB, patch)
2009-04-23 14:00 PDT, Boris Zbarsky [:bz] (still a bit busy)
bent.mozilla: review+
jonas: superreview+
Details | Diff | Splinter Review
1.9.0 fix (7.44 KB, patch)
2009-05-17 10:29 PDT, Boris Zbarsky [:bz] (still a bit busy)
dveditz: approval1.9.0.12+
Details | Diff | Splinter Review

Description User image Yongqian 2009-04-19 00:08:30 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv: Gecko/2009032609 Firefox/3.0.8 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv: Gecko/2009032609 Firefox/3.0.8 (.NET CLR 3.5.30729)

I am developing an extension which uses the jQuery library. The reverent aspect of the functionality can be thought of as being a simpler version of Greasemonkey. When a page loads, I execute jQuery in the context of the page and then execute a few other scripts that uses jQuery to modify the page DOM to provide the user with a few convenient features.

There is a bug however with Firefox such that executing jQuery causes certain pages to render twice, as if the entire page has been copy and pasted. I have provided a stripped down version of my extension which demonstrates this effect. In this version, jQuery is injected into the page in the form of a script tag but it also occurs if jQuery is executed by evalInSandbox.

I do not believe this is a jQuery bug since it works fine when I include it in the html file. It only triggers the bug when injected or eval'ed in evalInSandbox.

Reproducible: Always

Steps to Reproduce:
1. Install the provided xpi
2. open up the included html or go to
3. You may need to refresh the page
4. Everything is rendered twice!
Actual Results:  
Everything is rendered twice!

Expected Results:  
Everything is rendered only once.
Comment 1 User image Yongqian 2009-04-19 00:09:36 PDT
Created attachment 373549 [details]
xpi + html to reproduce bug
Comment 2 User image Brian Carpenter [:geeknik] 2009-04-19 00:18:06 PDT
I installed the extension under Firefox 3.5b4pre Build 20090418203959 and went to the forums and I did not see this behavior exhibited.  Could you attach a screenshot for those of us that don't have 3.0.8 installed?
Comment 3 User image Ria Klaassen (not reading all bugmail) 2009-04-19 09:39:16 PDT
Yeah, I was able to reproduce it (on Windows). I see repeated on the same page. :)
It has also a regression range:
So it is caused by Bug 396099 or Bug 392318.
Comment 4 User image Matthias Versen [:Matti] 2009-04-19 11:27:59 PDT
CC Mr 175+ Items for both bugs
Comment 5 User image Boris Zbarsky [:bz] (still a bit busy) 2009-04-23 13:23:12 PDT
Looks like a regression from bug 392318 somehow.
Comment 6 User image Boris Zbarsky [:bz] (still a bit busy) 2009-04-23 13:43:02 PDT
Here's the stack to us double-notifying:

#0  PresShell::ContentAppended (this=0x151ec00, aDocument=0x141e600, aContainer=0x17145620, aNewIndexInContainer=2) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/base/nsPresShell.cpp:4839
#1  0x11b79e1e in nsNodeUtils::ContentAppended (aContainer=0x17145620, aNewIndexInContainer=2) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsNodeUtils.cpp:119
#2  0x11ae4eb4 in nsContentSink::NotifyAppend (this=0x14e6c00, aContainer=0x17145620, aStartIndex=2) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsContentSink.cpp:1352
#3  0x11c9e585 in SinkContext::FlushTags (this=0x1d7e35b0) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/html/document/src/nsHTMLContentSink.cpp:1383
#4  0x11c9e67a in HTMLContentSink::FlushTags (this=0x14e6c00) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/html/document/src/nsHTMLContentSink.cpp:3219
#5  0x11c9e081 in HTMLContentSink::FlushPendingNotifications (this=0x14e6c00, aType=Flush_ContentAndNotify) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/html/document/src/nsHTMLContentSink.cpp:3198
#6  0x11b2dc54 in nsDocument::FlushPendingNotifications (this=0x141e600, aType=Flush_ContentAndNotify) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsDocument.cpp:6240
#7  0x11b19f83 in nsDocument::GetElementByIdInternal (this=0x141e600, aID=0x1d74e480) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsDocument.cpp:3761
#8  0x11b224e4 in nsDocument::GetElementById (this=0x141e600, aElementId=@0xbfffc118, aReturn=0xbfffc114) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/base/src/nsDocument.cpp:3785
#9  0x11cab629 in nsHTMLDocument::GetElementById (this=0x141e600, aElementId=@0xbfffc118, aReturn=0xbfffc114) at /Users/bzbarsky/mozilla/vanilla/mozilla/content/html/document/src/nsHTMLDocument.cpp:2321

The issue is that the script runs when DOMContentLoaded fires; at this point we have mWeakSink but not mParser.  So when the script first modifies the DOM (by inserting stuff before the <body>) and then triggers a flush, we try to flush the sink and end up notifying on the <body> a second time.  we end up with two frames for the <body> and everything under it.

There's a nice XXX comment in HTMLContentSink::DidBuildModel:

  // XXXbz I wonder whether we could End() our contexts here too, or something,
  // just to make sure we no longer notify...
Comment 7 User image Boris Zbarsky [:bz] (still a bit busy) 2009-04-23 14:00:25 PDT
Created attachment 374344 [details] [diff] [review]
Proposed fix

I'll try to think of a way to test this; I'd need a stylesheet that's guaranteed to load later than DOMContentLoaded fires...
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 2009-04-23 14:01:28 PDT
We should fix this on branches too.
Comment 9 User image Ben Turner (not reading bugmail, use the needinfo flag!) 2009-04-23 17:12:54 PDT
Comment on attachment 374344 [details] [diff] [review]
Proposed fix

makes sense to me, and looks good.
Comment 10 User image Daniel Veditz [:dveditz] 2009-05-01 10:43:23 PDT
bz: what's the security implication of rendering everything twice?

With code-freeze immanent and this not landed on trunk we're going to push this to .12 unless you say it's super scary and obvious to attackers.
Comment 11 User image Boris Zbarsky [:bz] (still a bit busy) 2009-05-01 11:00:33 PDT
The security implication is that you can end up with frames in the frame tree that are pointing to nodes that are no longer in the document.  Bug 490187 is a crash that's likely a result of something like that.

I _think_ on 1.9.0 those are mostly limited to null-deref issues, even for subdocument frames, but I haven't kept up on that.
Comment 12 User image Yongqian 2009-05-01 18:42:54 PDT
Is Bug 490187 exploitable? If so, maybe it should be marked as having security implications as well.
Comment 13 User image Boris Zbarsky [:bz] (still a bit busy) 2009-05-03 11:26:54 PDT
I don't think that null deref is exploitable per se, no.
Comment 14 User image Daniel Veditz [:dveditz] 2009-05-08 10:45:19 PDT
(In reply to comment #11)
> I _think_ on 1.9.0 those are mostly limited to null-deref issues, even for
> subdocument frames, but I haven't kept up on that.

We should just take this fix in 1.9.0 then. "mostly" null-derefs "I think" is not entirely reassuring :-)
Comment 15 User image Boris Zbarsky [:bz] (still a bit busy) 2009-05-08 11:50:50 PDT
Yeah, that's sorta my take on it too.  ;)
Comment 16 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2009-05-11 17:39:21 PDT
Jonas, can you review this?
Comment 17 User image Damon Sicore (:damons) 2009-05-15 11:34:36 PDT
Walking over to Jonas' desk with a stick.
Comment 18 User image Boris Zbarsky [:bz] (still a bit busy) 2009-05-17 10:15:51 PDT
Comment 19 User image Boris Zbarsky [:bz] (still a bit busy) 2009-05-17 10:29:44 PDT
Created attachment 377946 [details] [diff] [review]
1.9.0 fix
Comment 20 User image Boris Zbarsky [:bz] (still a bit busy) 2009-05-17 12:47:23 PDT
Comment 21 User image Samuel Sidler (old account; do not CC) 2009-05-18 17:18:32 PDT
Comment on attachment 377946 [details] [diff] [review]
1.9.0 fix

Boris, we're code frozen for Please re-nom for
Comment 22 User image Daniel Veditz [:dveditz] 2009-05-22 11:00:05 PDT
Comment on attachment 377946 [details] [diff] [review]
1.9.0 fix

Approved for, a=dveditz for release-drivers
Comment 23 User image Boris Zbarsky [:bz] (still a bit busy) 2009-06-01 09:11:42 PDT
Checking in content/base/src/nsContentSink.h;
/cvsroot/mozilla/content/base/src/nsContentSink.h,v  <--  nsContentSink.h
new revision: 1.45; previous revision: 1.44
Checking in content/html/document/src/nsHTMLContentSink.cpp;
/cvsroot/mozilla/content/html/document/src/nsHTMLContentSink.cpp,v  <--  nsHTMLContentSink.cpp
new revision: 3.811; previous revision: 3.810
Checking in content/xml/document/src/nsXMLContentSink.cpp;
/cvsroot/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v  <--  nsXMLContentSink.cpp
new revision: 1.450; previous revision: 1.449
Comment 24 User image Al Billings [:abillings] 2009-06-30 12:57:03 PDT
Verified for with testcase and xpi file. Fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2009063005 GranParadiso/3.0.12pre (.NET CLR 3.5.30729) and reproduces in 3.0.11.

Note You need to log in before you can comment on or make changes to this bug.