616397 - New Image() doesn't work in Greasemonkey scripts

Status: RESOLVED FIXED

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

Description

[dindog]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12
Build Identifier: Firefox 4.0b7

there are many scripts contained this:
     var img = new Image();

somehow firefox 4 drop support of this line, which makes tons of scripts stop wroking. It is made that way or a bug?


Reproducible: Always

Comment 1

[Boris Zbarsky [:bzbarsky]]

|new Image()| works fine for me as far as I can tell.  Can you please point to or attach (using the "Add an attachment" link) an actual page showing the problem?

Comment 2

[dindog]

Attachment: 2 fail js contain New Image

here are two greasemonkey javascript don't work properly.
both of them work fine in Firefox 3.6 but not Firefox 4.

after change

      new img(); ---->  document.createElement('img')

it work again
one js is show original image, another is a autopager-like.
you can test it in same page:
http://bbs.operachina.com/viewtopic.php?f=41&t=66751

Comment 3

[dindog]

Attachment: Edited JS which work on Firefox 4

almost the same 2 javascripts, only change a few line contain New Imgae();
which work fine on firefox 4b8.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> here are two greasemonkey javascript

Oh, greasemonkey!

If you load http://bbs.operachina.com/viewtopic.php?f=41&t=66751 and run this greasemonkey script:

  var img1 = new Image();
  var img2 = document.createElement('img');
  var same = (img1.ownerDocument == img2.ownerDocument);

then what's the value of |same| in Firefox 3.6?  What about Firefox 4?

Comment 5

[dindog]

(In reply to comment #4)
> > here are two greasemonkey javascript
> 
> Oh, greasemonkey!
> 
> If you load http://bbs.operachina.com/viewtopic.php?f=41&t=66751 and run this
> greasemonkey script:
> 
>   var img1 = new Image();
>   var img2 = document.createElement('img');
>   var same = (img1.ownerDocument == img2.ownerDocument);
> 
> then what's the value of |same| in Firefox 3.6?  What about Firefox 4?

I really don't know anything about javascripts, man. You tell me...lol.
I know nothing about scripts, but that would be strange if on one hand firefox 4 claims it take new image(); as it used to be, on the other hand just replaced new image(); to document.createElement('img'); make the scripts work again, right?

Comment 6

[Boris Zbarsky [:bzbarsky]]

ccing some greasemonkey folks.  Ehsan tried to reproduce this, but can't get user scripts to work at all with greasemonkey on trunk or b7...

Comment 7

[Boris Zbarsky [:bzbarsky]]

For what it's worth, I also can't add user scripts on trunk/b7: the greasemonkey service isn't found.  And the addon is not listed as compatible with those, of course; I had to override compat checking to even install it.

Comment 8

[Anthony Lieuallen]

Which version of greasemonkey?  I'd suggest trying the latest RC for our next release (released 0.8.6 is most definitely not compatible with 4.x): https://github.com/downloads/arantius/greasemonkey/greasemonkey-0.9.0-RC1.xpi

As to comment 4, I see true in "Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12", and on "Mozilla/5.0 (X11; Linux i686 on x86_64; rv:2.0b7) Gecko/20100101 Firefox/4.0b7" I see, in the error console:

Error: Out of Memory
Source File: file:///.../gm_scripts/616397_test/616397_test.user.js
Line: 8

(Line 8 being the "new Image()" line.)

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Which version of greasemonkey?

Whatever's on AMO (looks like 0.8.20100408.6).

Comment 10

[Boris Zbarsky [:bzbarsky]]

I'll try the rc, thanks.

Comment 11

[Boris Zbarsky [:bzbarsky]]

OK, I can confirm that in the rc |new Image()| in the greasemonkey script fails.

The reason it fails is that we land in the image constructor, try to find the relevant window by calling nsJSUtils::GetStaticScriptGlobal, walk up the parent chain to its root and find the sandbox object.  We get its private, which seems to be an nsGlobalWindow (?).  Then we try to QI it to nsIXPConnectWrappedNative and fail, and bail out, which ends up looking like us not having a window/document to work with, which causes NS_NewHTMLImageElement to return null, which looks like OOM to the caller.

Blake, is it expected to have the window itself as the private of the sandbox?  If so, should we change nsJSUtils::GetStaticScriptGlobal to handle this case?

Comment 13

[Johnny Stenback (:jst)]

Blocking on this regression.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: Make |new Image| work again in Greasemonkey scripts.

Comment 15

[Boris Zbarsky [:bzbarsky]]

I did also verify that the attached greasemonkey scripts work with that patch, and that the test throws the exception this bug is about without the patch.

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 499177 [details] [diff] [review]
Make |new Image| work again in Greasemonkey scripts.

Crossing my fingers.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Heh.  Against what?

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

Dunno exactly. GetStaticScriptGlobal has effects that filter up to a bunch of random places. I hope they all deal with this change correctly, is all.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Ah.  I think they should, yes.  In fact, this patch is needed to make error reporting for the web console work correctly too, in some cases...

Comment 20

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/78fd73ca849e