Closed Bug 525560 Opened 15 years ago Closed 14 years ago

Greasemonkey Extension Broken in Trunk 20091030 Build

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: murph.0912, Unassigned)

References

Details

The Greasemonkey v0.8.20090902.2 extension no longer works as of the Trunk (Minefield) 20091030 Nightly build. It produces the following error:

Error: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.evalInSandbox]
Source file: file:///C:(removed path details to user profile)/(user profile folder)/gm_scripts/(javascript folder)/(javascript).js
Line: 4294967295

Javascripts that used to work before in Trunk builds no longer work. Disabling both JIT content and JIT chrome had not effect on the functioning of Greasemonkey and its scripts.

Regression range (thanks to Alice0775):

Works:
http://hg.mozilla.org/mozilla-central/rev/f3e9ac90c086
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091029 Minefield/3.7a1pre ID:20091029141139

Broken:
http://hg.mozilla.org/mozilla-central/rev/091e9e8cba01
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091029 Minefield/3.7a1pre ID:20091029174823

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f3e9ac90c086&tochange=091e9e8cba01

I am filing this bug so that it can be determined if this is a browser side issue or an extension side issue.
Assignee: nobody → general
Component: Extension Compatibility → JavaScript Engine
Product: Firefox → Core
QA Contact: extension.compatibility → general
Greasemonkey is passing in 1.85 as the desired JS version

Components.utils.evalInSandbox(code, sandbox, maxJSVersion);

defined here:

const maxJSVersion = (function getMaxJSVersion() {
  // Default to version 1.6, which FF1.5 and later support.
  var jsVersion = 160;

  var jsds = Cc["@mozilla.org/js/jsd/debugger-service;1"].getService()
               .QueryInterface(Ci.jsdIDebuggerService);
  jsds.on();
  jsds.enumerateContexts({ enumerateContext: function(context) {
    if (context.version > jsVersion) jsVersion = context.version;
  }});
  jsds.off();

  return (jsVersion / 100).toString();
})();

but the changes to js/src/jsapi.cpp added ECMAv5 not 1.85 as the string, so this is probably a greasemonkey bug
Hi: greasemonkey maintainer here.

It looks like you're saying our "detect the highest supported JS version and ask for it" code is buggy.  Can you suggest a better way to accomplish that?

If I add a "dump(context.version+'\n');" into that loop in minefield, I see:

-------------------------------------
185
0
185
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
-------------------------------------

But in 3.5.x I see:

-------------------------------------
180
0
180
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
0
-------------------------------------

I certainly don't get all the zeros.  But the value certainly seems to have changed from 180 to 185.
It works for me if you simply leave out the maxJSVersion. Looking at the code, that makes it use the default version; would that suffice?

Components.utils.evalInSandbox(code, sandbox);//maxJSVersion);
http://github.com/greasemonkey/greasemonkey/issues/closed#issue/1026

Yes it works, but it uses the lowest-common-denominator version of JS.  Users want the newer JS engine available in FF 3.0, 3.5, etc.
(In reply to comment #6)
> http://github.com/greasemonkey/greasemonkey/issues/closed#issue/1026
> 
> Yes it works, but it uses the lowest-common-denominator version of JS.  Users
> want the newer JS engine available in FF 3.0, 3.5, etc.

I'm on WinXP. Isn't the JS engine what I have installed on my computer. I have Java SE 6 Update 16 on mine. Therefore, wouldn't only that be what is available to use?
(In reply to comment #7)
> I'm on WinXP. Isn't the JS engine what I have installed on my computer. I have
> Java SE 6 Update 16 on mine. Therefore, wouldn't only that be what is available
> to use?

Java SE 6 Update 16 is a *Java* runtime environment.  Java and JavaScript are not the same thing.  Gecko JavaScript has absolutely no relation to Sun Java.
OS: Windows XP → All
Hardware: x86 → All
Any progress on this bug?

I miss Greasemonkey.
(In reply to comment #10)
> Any progress on this bug?
> 
> I miss Greasemonkey.

Depending on what scripts you use, there may already be extensions that incorporate some or all of them. Better Gmail and Better G(oogle)Reader are two that I am aware of.

I found an online script compiler that will convert scripts into installable extensions. Go to http://arantius.com/misc/greasemonkey/script-compiler.
I have made 2 personal extensions to handle the two scripts that I was using with GM.
(In reply to comment #10)
> Any progress on this bug?
> 
> I miss Greasemonkey.

If you're a beta user comfortable with such things, find the code mentioned above (in components/greasemonkey.js) and remove it, replacing with:

  const maxJSVersion = '1.80';

And things will work for now.

To follow up the original issue:

The problem is that GM is passing the "wrong" value to evalInSandbox()'s third parameter.  The behavior we want is to get the highest JS engine supported by the user's browser.  Code above detected that, finding '1.80' on recent releases, and lower numbers on older ones.

What is this value, that should be passed as the third parameter, to get the newest JS features in the sandbox, for these versions of Firefox?  Is it still '1.80'?  Something else?
(In reply to comment #12)
> (In reply to comment #10)
> > Any progress on this bug?
> > 
> > I miss Greasemonkey.
> 
> If you're a beta user comfortable with such things, find the code mentioned
> above (in components/greasemonkey.js) and remove it, replacing with:
> 
>   const maxJSVersion = '1.80';
> 
> And things will work for now.
> 
> To follow up the original issue:
> 
> The problem is that GM is passing the "wrong" value to evalInSandbox()'s third
> parameter.  The behavior we want is to get the highest JS engine supported by
> the user's browser.  Code above detected that, finding '1.80' on recent
> releases, and lower numbers on older ones.
> 
> What is this value, that should be passed as the third parameter, to get the
> newest JS features in the sandbox, for these versions of Firefox?  Is it still
> '1.80'?  Something else?

I see the following:

const maxJSVersion = (function getMaxJSVersion() {
  // Default to version 1.6, which FF1.5 and later support.
  var jsVersion = 160;

  var jsds = Cc["@mozilla.org/js/jsd/debugger-service;1"].getService()
               .QueryInterface(Ci.jsdIDebuggerService);
  jsds.on();
  jsds.enumerateContexts({ enumerateContext: function(context) {
    if (context.version > jsVersion) jsVersion = context.version;
  }});
  jsds.off();

  return (jsVersion / 100).toString();
})();

Are you saying to replace the "160" above with "180"? (I just tried that and still get the same error.)

Or, are you saying to replace:

const maxJSVersion = (function getMaxJSVersion() {
  // Default to version 1.6, which FF1.5 and later support.
  var jsVersion = 160;

with:

  const maxJSVersion = '1.80';


?
I replaced the function with the constant, but it did not help.

This doesn't work as a script:
document.body.innerHTML=document.body.innerHTML.replace(/foo1/gi, "foo2");

but DOES work on the location bar:
javascript:document.body.innerHTML=document.body.innerHTML.replace(/foo1/gi, "foo2");
Ray: The entire thing, the second option you listed.

Earl: This is not related to this bug.  I'd suggest you seek support at the greasemonkey-users list.  Also: Works fine for me.
Anthony: it worked with nightly builds fine until two-three days ago.
(In reply to comment #12)

I replaced that function with the line you gave, and GM scripts still give the NS_ERROR_ILLEGAL_VALUE error.
Changing code mentioned in #13 to const maxJSVersion = '1.80'; creates even more errors for me and doesn't fix Greasemonkey.
I replaced this section in greasemonkey.js:

const maxJSVersion = (function getMaxJSVersion() {
  // Default to version 1.6, which FF1.5 and later support.
  var jsVersion = 160;

  var jsds = Cc["@mozilla.org/js/jsd/debugger-service;1"].getService()
               .QueryInterface(Ci.jsdIDebuggerService);
  jsds.on();
  jsds.enumerateContexts({ enumerateContext: function(context) {
    if (context.version > jsVersion) jsVersion = context.version;
  }});
  jsds.off();

  return (jsVersion / 100).toString();
})();

With:

  const maxJSVersion = 1.80;

And, GM now works, again, in Minefield.
I tried that Ray. I've tried setting it to 1.60, 1.70 and 1.80 and I still get a ton of errors in the Error log along with a bunch of scripts that don't work. :)

Error: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.evalInSandbox]
Source File: somerandomgreasemonkeyscript.js
Line: 4294967295

Funny. That script only has like 20 lines.
I am getting this error:

Error: parent is null
Source file: file:///C:(details removed)/components/greasemonkey.js
Line: 399

but so far, my scripts are working.


Did you remove from the recompiled xpi and bak files generated while editing?

Did you remove all of this:

(function getMaxJSVersion() {
  // Default to version 1.6, which FF1.5 and later support.
  var jsVersion = 160;

  var jsds = Cc["@mozilla.org/js/jsd/debugger-service;1"].getService()
               .QueryInterface(Ci.jsdIDebuggerService);
  jsds.on();
  jsds.enumerateContexts({ enumerateContext: function(context) {
    if (context.version > jsVersion) jsVersion = context.version;
  }});
  jsds.off();

  return (jsVersion / 100).toString();
})()

 and replace it with 1.80?
(In reply to comment #21)

I downloaded a fresh copy of the xpi and edited that. (Replacing the entire function with 1.80 ((although it should be '1.80', as the function called toString at the very end)) ).

I then installed it in Minefield.

Still doesn't work.
Oh, hey, I tried it again. Doing it without the quotes worked.
(In reply to comment #22)
> (In reply to comment #21)
> 
> I downloaded a fresh copy of the xpi and edited that. (Replacing the entire
> function with 1.80 ((although it should be '1.80', as the function called
> toString at the very end)) ).
> 
> I then installed it in Minefield.
> 
> Still doesn't work.

I tried 1.80 WITH the "'" around it and got the original error. After removing the "'", I only get the "parent is null" error and GM appears to be working.

I wonder if it would be OK to post an attachment with the content of my modified greasemonkey.js file?
(In reply to comment #24)
> (In reply to comment #22)
> > (In reply to comment #21)
> > 
> > I downloaded a fresh copy of the xpi and edited that. (Replacing the entire
> > function with 1.80 ((although it should be '1.80', as the function called
> > toString at the very end)) ).
> > 
> > I then installed it in Minefield.
> > 
> > Still doesn't work.
> 
> I tried 1.80 WITH the "'" around it and got the original error. After removing
> the "'", I only get the "parent is null" error and GM appears to be working.
> 
> I wonder if it would be OK to post an attachment with the content of my
> modified greasemonkey.js file?

Why, we need just a patch, not the whole content ;)
Indeed, though it is strange (see return (jsVersion / 100).toString()), if maxJSVersion is a real constant and not a string one, my scripts work again.
The last sixteen comments (possibly more, possibly less, I started skimming after the first couple of those appeared unrelated to fixing this bug) are not helpful in determining how to fix this bug.  They belong on a Greasemonkey mailing list or in a Greasemonkey IRC channel or somewhere else of that nature, not here.   Bugzilla is for development work, not extension support.

What's the reason you can't just hardcode a max-version list directly and then iterate from greatest to least through it to determine the max version?  It's not like you aren't updating the extension fairly often anyway.  SpiderMonkey hackers aren't great fans of JS versioning due to the code complexity it introduces, and we'd rather not make things more complicated if at all possible.
I think they know that "maxJSVersion = 1.80;" would get it working again with the old JS version.

That doesn't solve the problem for getting Greasemonkey to work with Minefield's new version.
> The last sixteen comments (possibly more, possibly less, I started skimming
after the first couple of those appeared unrelated to fixing this bug) are not
helpful in determining how to fix this bug.

Second.  This is confused by a long-standing Mozilla bug: eval() (and thus evalInSandbox() ) errors are reported as coming from the script that runs the eval.  So often, users think an error in a script comes from GM.

> What's the reason you can't just hardcode a max-version list directly and then
iterate from greatest to least through it to determine the max version?

Based on user requests, we decided to pass the third parameter to evalInSandbox() so that the script would run with the newest JS engine available in _that_ browser.  I.E. Someone still running Firefox 1.5 (bad example, but I know the right value here:) will get JS 1.6.  One running Firefox 3.5 will get JS 1.8.

Are you saying that we should hard-code a FF version -> JS version mapping?
Ok, here is a patch for greasemonkey, could prove useful for somebody: http://sprunge.us/YiXc
(In reply to comment #29)
> Are you saying that we should hard-code a FF version -> JS version mapping?

Yes, I think so.  It's easy work that coincides with the major-version update cycle when you're already updating little things anyway, and it doesn't require a lengthy series of upgrades before everyone's using Firefox versions which support some mythical version-exposure mechanism (back- and forwards-compatible makes for simplicity).
|jsds.enumerateContexts()| ends up calling into [1].  This suggests to me that this code relies on having a JSContext created explicitly with the latest JS version.  I don't know why that would be true, but apparently it is.  I'm not sure that's a good assumption to make.

The JS debugger service is also broken in that it hands out to JS code the numeric version value of the C++ |enum JSVersion| that's only used symbolically in jsapi.  It should be handing out to JS the stringified version of this enum, see [2].

I think I favor Jeff's suggested fix, but want to point out that there's a relatively simple backwards-compatible version-exposure mechanism
 (1) Add |JS_LatestestVersion()| to jsapi
 (2) Expose that as a vanilla JS property on something ("something" == jsd doesn't feel right to me, but would work)
 (3) Change Greasemonkey's version detection code to
  latestVersion = something.latestVersion;
  if (undefined == latestVersion)  latestVersion = "1.80";

[1] https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_ContextIterator
[2] https://developer.mozilla.org/en/SpiderMonkey/JSAPI_Reference/JS_VersionToString
(In reply to comment #32)
>  (3) Change Greasemonkey's version detection code to
>   latestVersion = something.latestVersion;
>   if (undefined == latestVersion)  latestVersion = "1.80";
> 

Oops sorry, if the |latestVersion| is undefined this code should fall back on the previous algorithm.  But like I said, I favor Jeff's suggestion ;).
Hi -- other Greasemonkey maintainer here. Our goal here is that Greasemonkey evaluates user script code using whatever javascript features would be available for a script tag in the page (so javascript wizards can start using let statements and similar goodies coming up in the future, ideally without filing any tickets on Greasemonkey or here).

evalInSandbox used to do that until FF 3.0 (running javascript 1.6).

From 3.5 onwards, to get the wanted behaviour, a third parameter with a magic constant "1.8" (currently) is required.

A most delightful conclusion for this ticket would be the invention of some probable constant (Components.utils.evalInSandbox.maxJSVersion, or similar), that we could recognize, when available, and pass as the third argument, rather than forever after doing browser sniffing and maintaining a lookup table of what magic constant to pass.

For now, upcoming Greasemonkey builds know the present incantation, as of http://github.com/greasemonkey/greasemonkey/commit/0c4994ffc37f6be61514481980d4437f8a936e2f
(In reply to comment #32)

Sorry to go all verbose when our ideal solution was already suggested:

> I think I favor Jeff's suggested fix, but want to point out that there's a
> relatively simple backwards-compatible version-exposure mechanism
>  (1) Add |JS_LatestestVersion()| to jsapi
>  (2) Expose that as a vanilla JS property on something ("something" == jsd
> doesn't feel right to me, but would work)
>  (3) Change Greasemonkey's version detection code to
>   latestVersion = something.latestVersion;
>   if (undefined == latestVersion)  latestVersion = "1.80";

We're mostly interested in this kind of feature for forwards-compatibility -- our current code has the "1.6" / "1.8" guess wired to version sniffing already. (There is no preference on our part to make jsd the "something" above; the only reason we dug in there was that it was the only way we came up with that seemed to expose the requested info about the available javascript engine.)
Fixed in Greasemonkey 0.8.5.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.