What we really want to happen here is for JSOP_GLOBALTHIS to point to the window inside these sandboxes, right?

We already have provisions for a hacked-up JSOP_GLOBALTHIS when non-syntactic scopes are involved, but that would add some performance annoyance here that is probably not desirable.

Maybe we could add some other way to tell a JSScript that its JSOP_GLOBALTHIS should be a different object (in this case the proto of the sandbox global, I believe)?

There would still be issues with var x putting the variable on the global, not the window, I'd think, but in general it might make us more compatible than what we have now, perhaps.

You can tell JSOP_GLOBALTHIS to use a different object if you expose something like LexicalEnvironmentObject::setWindowProxyThisValue that doesn't assert is a window proxy. I believe the engine is consistent enough about the 'this' due to JSM support. (Well, 80% sure, but I think I had to normalize all this to allow JSMs to share a global).

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #45)

There would still be issues with var x putting the variable on the global, not the window, I'd think, but in general it might make us more compatible than what we have now, perhaps.

Wouldn't making window globalThis make all top level vars go to it?

Unfortunately global-this is distinct from the-root-of-the-scope-chain in the spec. Var definitions are not the same under the hood as this.x unfortunately. Normal content uses the same meaning for both, but the proposal of tweaking JSOP_GLOBALTHIS would only touch the one case.

The thing is that we'd actually want all vars to go to it, and also all unbound variable assignments. The fact that they don't is one of the more common complaints associated with this bug.

(And my proof-of-concept patch fixed the JSOP_GLOBALTHIS and top-level var declaration problems, but not the unbound variable assignments. But also it introduced nonsyntactic scope chains, which probably destroy the performance of content scripts, which is already not great at best.)

Hey Kris, earlier comments mention same-compartment realms (bug 1357862). That's done now but how does it relate to this bug, can we take advantage of that somehow?

(In reply to Jan de Mooij [:jandem] from comment #51)

Hey Kris, earlier comments mention same-compartment realms (bug 1357862). That's done now but how does it relate to this bug, can we take advantage of that somehow?

It's sort of orthogonal to this bug, but somewhat related. We need same-compartment realms to let multiple content script sandboxes see the expandos each other has added to a given window object, which should probably get a separate bug. But we need to do other things to top-level var assignments and this.foo assignments to go to the window object, too.

(In reply to Ted Campbell [:tcampbell] from comment #46)

You can tell JSOP_GLOBALTHIS to use a different object if you expose something like LexicalEnvironmentObject::setWindowProxyThisValue that doesn't assert is a window proxy. I believe the engine is consistent enough about the 'this' due to JSM support. (Well, 80% sure, but I think I had to normalize all this to allow JSMs to share a global).

So, next step here would be adding a public LexicalEnvironmentObject::setGlobalThisValue method, hooking it up to a new option for Cu.Sandbox, and just testing if anything burns up?

This issue breaks Symbol too, so that const thing = window[Symbol.for(secret)] || (window[Symbol.for(secret)] = thing()) produces undefined every single time.

In an attempt to secure our globals, sandboxing once functions such as getComputedStyle, or even setTimeout, Firefox only, as Chrome is just fine, is incapable of running those global functions without an explicit window context, which breaks compatibility with ECMAScript, as setTimeout called via const {setTimeout} = window should never break by specs.

This is a very bad bug to face, as everything happening in content scripts become unpredictable, so that the sandbox seems to backfire all over the place, and developers waste a lot of time figuring out why Math !== window.Math, or other completely unexpected stuff.

This is also a 5 years old bug, so I hope there is a resolution, as extensions trying to meomize once globals to improve, in a way or another, their confidence in using not-tainted globals, will harakiri instead due an un-trustable environment created by the browser itself.

Thanks for hopefully taking an action on this Firefox only issue.

Best Regards.

P.S. poisoning/tainting the whole environment in the name of better security has been backfiring on environment reliability for 5+ years. I think we need better code review before publishing on the store, or some sort of trust index per extension (i.e. there's a company behind), instead of poisoning everything underneath, IMHO. Please drop these side-effecting wrappers that also likely slow down every extension execution, and are incapable of creating a 1:1 real browser environment, thanks.

In an attempt to secure our globals, sandboxing once functions such as getComputedStyle, or even setTimeout, Firefox only, as Chrome is just fine,

I'm not quite sure I understand the problem. Extensions already run in an isolated context. Untrusted page scripts cannot interfere with the getComputedStyle or setTimeout functions used by extension scripts.

This issue breaks Symbol too, so that const thing = window[Symbol.for(secret)] || (window[Symbol.for(secret)] = thing()) produces undefined every single time.

Have you tried globalThis instead?

I'm not quite sure I understand the problem. Extensions already run in an isolated context

It's broken. If we extract any global method and invoke it, it breaks. We had to bind the window per each extracted method via setTimeout.bind(window) and similar.

The env is not mutable from the browser, the env itself though, has bad wrappers around global functionalities.

Have you tried globalThis instead?

To polyfill globalThis we need to use window, as we target down to Firefox 52 ESR where no globalThis exists.

Almost 6 years, any process on this issue?

For example, when using 3rd party library in content scripts:

// manifest.json
{
"js": [
    "react.js",
    "react-dom.js",
    "jss.js",
    "jss-preset-default.js",
    "content.js"
],

react.js / react-dom.js expose the exports object to window, and jss.js / jss-preset-default.js expose to globalThis. And also Firefox lack of debugging content scripts feature the DevTools, so it's hard to found out where the library object before reading the library source.

This rather painful port Chrome extension to Firefox because the difference behavior.

(In reply to muzuiget from comment #60)

And also Firefox lack of debugging content scripts feature the DevTools, so it's hard to found out where the library object before reading the library source.

You can use the "browser content toolbox" for content scripts and about:debugging for the background page.

The severity field for this bug is relatively low, S3. However, the bug has 11 duplicates.
:robwu, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

I submitted a PR to document the current behavior at https://github.com/mdn/content/pull/24215