In nightly, @require doesn't work as intended #1592

Closed
arantius opened this Issue Jul 19, 2012 · 3 comments

Comments

Projects
None yet
1 participant
@arantius
Collaborator

arantius commented Jul 19, 2012

Greasemonkey: 2012.07.16.nightly
Test script: https://gist.github.com/3143842

Will log:

Error: library is not defined
Source File: file:///.../gm_scripts/Require_Test/require-test.user-1.js
Line: 12

This has got to be due to the recent injection change to use line numbers (#1404) which eval'ed each section of the script separately .. in its own anonymous wrapper, separating the namespaces, which was not intended.

arantius added a commit to arantius/greasemonkey that referenced this issue Jul 23, 2012

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Aug 1, 2012

Collaborator

I think the only reason we still anon-wrap by default is the old "var sidebar" issue:
https://groups.google.com/d/topic/greasemonkey-dev/A6RwuonRvXw/discussion

Way back in 2008 we removed it, hit that, put it back and haven't touched it. We still wrap if there is a bare return, otherwise the script just fails to run -- in this eval with lines scheme, just the one (require or script) thing would fail to run.

So right now I'm trying to reproduce the "var sidebar" issue, goal being finding a different solution to that, and removing the anon wrapper by default. So I made https://gist.github.com/3228230 and tried:

Greasemonkey development head, Firefox 15 beta: Runs fine.
Greasemonkey 0.9.20, Firefox 3.6.28: Runs fine.
Greasemonkey 0.8.20100408.6, Firefox 2.0.0.20: Finally a failure:

Error: Illegal value
Source File: file:///.../gm_scripts/sidebar_error/sidebar_error.user.js
Line: 57

Line 57 is var sidebar;. So I continued narrowing:

Greasemonkey 0.8.20100408.6, Firefox 3.6.28: Fails, same.

So this is only broken in Greasemonkey 0.8? Maybe the don't wrap "temporary workaround" won't be so temporary.

Collaborator

arantius commented Aug 1, 2012

I think the only reason we still anon-wrap by default is the old "var sidebar" issue:
https://groups.google.com/d/topic/greasemonkey-dev/A6RwuonRvXw/discussion

Way back in 2008 we removed it, hit that, put it back and haven't touched it. We still wrap if there is a bare return, otherwise the script just fails to run -- in this eval with lines scheme, just the one (require or script) thing would fail to run.

So right now I'm trying to reproduce the "var sidebar" issue, goal being finding a different solution to that, and removing the anon wrapper by default. So I made https://gist.github.com/3228230 and tried:

Greasemonkey development head, Firefox 15 beta: Runs fine.
Greasemonkey 0.9.20, Firefox 3.6.28: Runs fine.
Greasemonkey 0.8.20100408.6, Firefox 2.0.0.20: Finally a failure:

Error: Illegal value
Source File: file:///.../gm_scripts/sidebar_error/sidebar_error.user.js
Line: 57

Line 57 is var sidebar;. So I continued narrowing:

Greasemonkey 0.8.20100408.6, Firefox 3.6.28: Fails, same.

So this is only broken in Greasemonkey 0.8? Maybe the don't wrap "temporary workaround" won't be so temporary.

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Aug 1, 2012

Collaborator

More test cases, separate files, still all in https://gist.github.com/3228230

Firefox 2.0.0.20, Greasemonkey 0.8.20100408.6:
"var x" and "var x equals" give illegal value. "x equals" gives component unavailable.

Firefox 3.6.28, Greasemonkey 0.8.20100408.6:
"var x" and "var x equals" give "redeclaration of const" for navigator and then window. "x equals" gives component unavailable.
Comment out the redeclared consts and we get illegal value like Firefox 2.

Firefox 3.6.28, Greasemonkey 0.9.0:
"x equals" and "var x equals" give component not available. "var x" runs fine (assuming navigator/window are commented).

Firefox 4.0.1, Greasemonkey 0.9.0:
No errors logged. Log lines at the bottom of all three versions complete successfully.

Firefox 15 beta, Greasemonkey 0.9.0:
No errors logged. Log lines at the bottom of all three versions complete successfully.

Firefox 15 beta, Greasemonkey development head:
No errors logged. Log lines at the bottom of all three versions complete successfully.

Failure messages look like:

Error: Illegal value
Source File: file:///.../gm_scripts/sidebar_error/var_x.user.js
Line: 60

and

Error: Component is not available
Source File: file:///.../gm_scripts/x_equals/x_equals.user.js
Line: 16    

This is a reproduction of both style of errors mentioned in the history of this bug, in various Firefox/Greasemonkey versions. But never in recent versions of both. This makes me much more confident that the bug is squashed enough to consider fully dead for 1.0 development.

Collaborator

arantius commented Aug 1, 2012

More test cases, separate files, still all in https://gist.github.com/3228230

Firefox 2.0.0.20, Greasemonkey 0.8.20100408.6:
"var x" and "var x equals" give illegal value. "x equals" gives component unavailable.

Firefox 3.6.28, Greasemonkey 0.8.20100408.6:
"var x" and "var x equals" give "redeclaration of const" for navigator and then window. "x equals" gives component unavailable.
Comment out the redeclared consts and we get illegal value like Firefox 2.

Firefox 3.6.28, Greasemonkey 0.9.0:
"x equals" and "var x equals" give component not available. "var x" runs fine (assuming navigator/window are commented).

Firefox 4.0.1, Greasemonkey 0.9.0:
No errors logged. Log lines at the bottom of all three versions complete successfully.

Firefox 15 beta, Greasemonkey 0.9.0:
No errors logged. Log lines at the bottom of all three versions complete successfully.

Firefox 15 beta, Greasemonkey development head:
No errors logged. Log lines at the bottom of all three versions complete successfully.

Failure messages look like:

Error: Illegal value
Source File: file:///.../gm_scripts/sidebar_error/var_x.user.js
Line: 60

and

Error: Component is not available
Source File: file:///.../gm_scripts/x_equals/x_equals.user.js
Line: 16    

This is a reproduction of both style of errors mentioned in the history of this bug, in various Firefox/Greasemonkey versions. But never in recent versions of both. This makes me much more confident that the bug is squashed enough to consider fully dead for 1.0 development.

@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Aug 1, 2012

Collaborator

If you do @grant none, then in GM dev head, you'll see:

  Error: setting a property that has only a getter
  Source file: file:///.../gm_scripts/X_Equals/x-equals.user.js
  Line: 15

Only for "x equals". For most of the IDL properties. But the error message is extremely clear and points to the specific line in question. I'm comfortable leaving that much; use of these variable names isn't super common. And of course this is the same behavior you'd see in standard JS in a page.

Collaborator

arantius commented Aug 1, 2012

If you do @grant none, then in GM dev head, you'll see:

  Error: setting a property that has only a getter
  Source file: file:///.../gm_scripts/X_Equals/x-equals.user.js
  Line: 15

Only for "x equals". For most of the IDL properties. But the error message is extremely clear and points to the specific line in question. I'm comfortable leaving that much; use of these variable names isn't super common. And of course this is the same behavior you'd see in standard JS in a page.

arantius added a commit to arantius/greasemonkey that referenced this issue Aug 1, 2012

Never anon wrap scripts, except in return outside function case.
Also remove "@unwrap" as it now serves no purpose.
Add deprecation warning about return-outside-function.  (You can do this with your own anon wrapper, one day I'd like to remove it altogether.)

Refs #1592

@arantius arantius closed this Aug 7, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment