Closed Bug 616397 Opened 9 years ago Closed 9 years ago

New Image() doesn't work in Greasemonkey scripts

Categories

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

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla2.0b9
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: dindog, Assigned: bzbarsky)

References

Details

(Keywords: regression)

Attachments

(3 files)

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
|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?
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
almost the same 2 javascripts, only change a few line contain New Imgae();
which work fine on firefox 4b8.
> 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?
(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?
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...
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.
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.)
> Which version of greasemonkey?

Whatever's on AMO (looks like 0.8.20100408.6).
I'll try the rc, thanks.
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?
Blocks: compartments
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Component: General → DOM
Ever confirmed: true
QA Contact: general → general
Summary: New Image(); don't work in Firefox 4 → New Image() doesn't work in Greasemonkey scripts
Duplicate of this bug: 616663
Blocking on this regression.
Assignee: nobody → mrbkap
blocking2.0: ? → betaN+
Assignee: mrbkap → bzbarsky
Priority: -- → P1
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.
Whiteboard: [need review]
Comment on attachment 499177 [details] [diff] [review]
Make |new Image| work again in Greasemonkey scripts.

Crossing my fingers.
Attachment #499177 - Flags: review?(mrbkap) → review+
Heh.  Against what?
Whiteboard: [need review] → [need landin]
Whiteboard: [need landin] → [need landing]
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.
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...
Pushed http://hg.mozilla.org/mozilla-central/rev/78fd73ca849e
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [need landing]
Target Milestone: --- → mozilla2.0b9
Blocks: 630386
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.