Last Comment Bug 707749 - I need to click on the bookmarklet twice to make it work. If document.write is included in the bookmarklet (involves navigator reparenting)
: I need to click on the bookmarklet twice to make it work. If document.write i...
Status: VERIFIED FIXED
[qa!]
: regression
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: 11 Branch
: All All
: -- normal with 1 vote (vote)
: mozilla12
Assigned To: Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017)
:
: Andrew Overholt [:overholt]
Mentors:
: 713002 (view as bug list)
Depends on:
Blocks: 700202
  Show dependency treegraph
 
Reported: 2011-12-05 10:29 PST by Alice0775 White
Modified: 2012-02-24 08:52 PST (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified


Attachments
Proposed fix (5.08 KB, patch)
2011-12-23 06:50 PST, Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017)
jst: review+
christian: approval‑mozilla‑aurora+
mrbkap: checkin+
Details | Diff | Splinter Review

Description Alice0775 White 2011-12-05 10:29:10 PST
Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/1bd7482ad4d1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111205 Firefox/11.0a1 ID:20111205031110

I need to click on the bookmarklet twice to make it work. If document.write is included in the bookmarklet.


Reproducible: Always

Steps to Reproduce:
1. Start Firefox with clean profile
2. Bookmark javascript:document.write("aaaaaaa");document.close();
3. Open any web page (ex. http://www.mozilla.org/en-US/about/ )
4. Click the bookmarklet created in step2

5. Click again the bookmarklet created in step2


Actual Results:
  *Nothing happen at Step4, and And an error message is shown in Error console.

  Error: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED)
  [nsIDOMHTMLDocument.write]
  Source file: javascript:document.write("aaaaaaa");document.close();
  Line: 1

  *However successfully executed at Step5.

Expected Results:
  The bookmarklet should be executed at Step4 as well as Step5.


Regression window(m-c)
Works:
http://hg.mozilla.org/mozilla-central/rev/9740118b9dcc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111203 Firefox/11.0a1 ID:20111203031109
Fails:
http://hg.mozilla.org/mozilla-central/rev/a68c96c1d8e0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111203 Firefox/11.0a1 ID:20111203031117
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9740118b9dcc&tochange=a68c96c1d8e0


Regression window(m-i)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/8304db7e46bb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111202 Firefox/11.0a1 ID:20111202212018
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2cddf1a5d9e2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111202 Firefox/11.0a1 ID:20111202221915
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8304db7e46bb&tochange=2cddf1a5d9e2

Suspected: Bug 700202
Comment 1 Alice0775 White 2011-12-05 10:40:35 PST
First report is in mozillazine( http://forums.mozillazine.org/viewtopic.php?p=11531009#p11531009 )
Comment 2 Bobby Holley (:bholley) (busy with Stylo) 2011-12-05 11:08:21 PST
This might indeed be a regression from sfink's patch. But since this patch just makes us propagate exceptions correctly, it's likely that it's just exposing a latent bug. sfink, want to take a look?
Comment 3 Alice0775 White 2011-12-08 20:17:42 PST
This problem affects not only execution of bookmarklt but also execution  of web page Javascript.

After I failed to execute the bookmarklet( i.e. I click the bookmarklet once) on http://www.mozilla.org/projects/firefox/prerelease.html. 
Errors appear in error console when mouse hover web page.

Error: attempt to run compile-and-go script on a cleared scope
Source file: chrome://global/content/bindings/videocontrols.xml
Line: 454

Error: attempt to run compile-and-go script on a cleared scope
Source file: http://mozcom-cdn.mozilla.net/org/script/1.0/jquery-1.4.3.min.js
Line: 56

etc...

And on https://docs.google.com/
Errors appear in error console when mouse hover web page.
And checkboxes no longer work until I load the page again.


Error: attempt to run compile-and-go script on a cleared scope
Source file: https://docs.google.com/static/doclist/client/js/4058151398-doclist_modularized-gecko_core.js
Line: 181
Comment 4 Steve Fink [:sfink] [:s:] 2011-12-09 10:37:08 PST
(In reply to Bobby Holley (:bholley) from comment #2)
> This might indeed be a regression from sfink's patch. But since this patch
> just makes us propagate exceptions correctly, it's likely that it's just
> exposing a latent bug. sfink, want to take a look?

What I'm seeing is in dom/base/nsDOMClassInfo.cpp:

  nsGlobalWindow *win = static_cast<nsGlobalWindow*>(nav->GetWindow());
  if (!win) {
    NS_WARNING("Refusing to create a navigator in the wrong scope");

    return NS_ERROR_UNEXPECTED;
  }

that error is getting caught, and now nsXPConnect::MoveWrappers() no longer ignores it.

I see bugs@pettay.fi is CC'd. smaug, what's the right way to handle this?

There's also the cleared scope error. I'm not sure whether it's breaking anything or not. It's triggered by a script in the page you started from (in my case, some horribly mangled URL from www.google.com, being invoked on an HTMLInputElement due to a blur event, but Alice0775 has other examples in comment 3.) It's probably something about the ordering of actions when using document.write from a bookmarklet. Tracing through with the debugger, though, it seems like this error will be ignored and so is probably irrelevant to the behavior change in this bug. (It ends setting a flag on the event saying that it provoked an exception, but that's it.)
Comment 5 Olli Pettay [:smaug] 2011-12-15 17:40:50 PST
So what part of https://bugzilla.mozilla.org/show_bug.cgi?id=700202 is causing the problem?
Comment 6 Olli Pettay [:smaug] 2011-12-15 17:46:42 PST
I would probably back out patches for bug 700202, and try to fix it so that this bug doesn't regress.
Comment 7 Olli Pettay [:smaug] 2011-12-15 17:47:56 PST
(and add an automated testcase for this)
Comment 8 Olli Pettay [:smaug] 2011-12-15 18:02:36 PST
Did bug 690952 change some behavior here?
I think 
nsGlobalWindow *win = static_cast<nsGlobalWindow*>(nav->GetWindow());
was added in that bug.
Comment 9 Olli Pettay [:smaug] 2011-12-15 18:05:16 PST
My _guess_ is that something like this happens:
inner window A is created, and Navigator for it, but not js wrapper for Navigator.
Then document.write destroys A and creates a new inner window B, and for some reason after that
the Navigator for A gets it js wrapper created.
Comment 10 Steve Fink [:sfink] [:s:] 2011-12-16 10:46:32 PST
(In reply to Olli Pettay [:smaug] from comment #5)
> So what part of https://bugzilla.mozilla.org/show_bug.cgi?id=700202 is
> causing the problem?

This part:

     nsCOMPtr<nsIScriptGlobalObject> newScope(do_QueryReferent(mScopeObject));
     if (oldScope && newScope != oldScope) {
-      nsContentUtils::ReparentContentWrappersInScope(cx, oldScope, newScope);
+      rv = nsContentUtils::ReparentContentWrappersInScope(cx, oldScope, newScope);
+      NS_ENSURE_SUCCESS(rv, rv);
     }
   }

Previously, ReparentContentWrappersInScope would fail but the error would be ignored. Now it aborts out of nsHTMLDocument::Open.

We could back out bug 700202 to fix this bug temporarily if the real problem is too hard to fix for now. Obviously, it would be better to prevent ReparentContentWrappersInScope() from failing, or at least detect and suppress the error closer to its source when it is known to be ignorable. (For correctness, the exception needs to be cleared on the JSContext if it's going to be ignored. Otherwise it can interfere with the next JS code run with that context.)
Comment 11 David Mandelin [:dmandelin] 2011-12-21 17:15:20 PST
(In reply to Steve Fink [:sfink] from comment #10)
> We could back out bug 700202 to fix this bug temporarily if the real problem
> is too hard to fix for now.

This could break bookmarklets and affects 11, which is now in Aurora, so I'd like to get it fixed promptly. Do you (and smaug) think it can be fixed within a few weeks, or would you prefer to back out bug 700202?
Comment 12 Olli Pettay [:smaug] 2011-12-21 17:17:34 PST
So did anyone check why and where is the wrong navigator object created?
Is comment 9 right?
Comment 13 Olli Pettay [:smaug] 2011-12-22 09:59:19 PST
*** Bug 713002 has been marked as a duplicate of this bug. ***
Comment 14 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2011-12-23 06:50:49 PST
Created attachment 584052 [details] [diff] [review]
Proposed fix

So, the problem is that we've called FreeInnerObjects by the time the document.write reparenting happens. FreeInnerObjects invalidates the navigator, meaning that we can't even call PreCreate on it without throwing. The easiest fix is to move the navigator object explicitly in SetNewDocument so that it continues to work. I think that given the semantics of document.open, this is correct.
Comment 15 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2011-12-23 06:55:43 PST
Oh, the other key is that navigator has to already have been created when document.open is called.
Comment 16 Olli Pettay [:smaug] 2011-12-24 14:59:54 PST
Blake, does the patch fix also bug 713002?
Comment 17 chado 2011-12-26 00:26:19 PST
(In reply to Olli Pettay [:smaug] from comment #16)
> Blake, does the patch fix also bug 713002?

Sorry, I'm not Blake.
I've just tried the proposed patch (attachment#584052 [details] [diff] [review]) to the source 
http://hg.mozilla.org/mozilla-central/rev/798b00a6fe29 locally on Ubuntu.

Things looking good about both of bug 707749#c0 and bug 713002#c0 .
Comment 18 Johnny Stenback (:jst, jst@mozilla.com) 2011-12-26 02:59:05 PST
Comment on attachment 584052 [details] [diff] [review]
Proposed fix

Looks good, r=jst
Comment 19 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2011-12-28 06:22:40 PST
Comment on attachment 584052 [details] [diff] [review]
Proposed fix

https://hg.mozilla.org/integration/mozilla-inbound/rev/e4dd116797dc
Comment 20 Marco Bonardo [::mak] 2011-12-29 03:17:22 PST
https://bugzilla.mozilla.org/show_bug.cgi?id=707749
Comment 21 Steve Fink [:sfink] [:s:] 2011-12-29 10:33:58 PST
Comment on attachment 584052 [details] [diff] [review]
Proposed fix

This is needed to fix the regression on bookmarklets in Aurora.
Comment 22 christian 2011-12-29 14:47:54 PST
Comment on attachment 584052 [details] [diff] [review]
Proposed fix

[triage comment]
Approved for Aurora. JS regression in Fx11 that affects web content and bookmarklets.
Comment 23 Peter van der Woude [:Peter6] 2011-12-30 07:51:48 PST
Mozilla/5.0 (Windows NT 5.1; rv:12.0a1) Gecko/20111230 Firefox/12.0a1 ID:20111230031151
Comment 24 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2012-01-11 09:33:26 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/ea6c90cdb525
Comment 25 Paul Silaghi, QA [:pauly] 2012-02-24 08:52:34 PST
Bookmarklet is opened after the first click, no errors in Error Console.

Verified fixed on Firefox 11b4:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0

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