Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pollution in global scope #44

Closed
jrummell-chromium opened this issue Apr 27, 2022 · 4 comments · Fixed by #61 or #49
Closed

Pollution in global scope #44

jrummell-chromium opened this issue Apr 27, 2022 · 4 comments · Fixed by #61 or #49
Assignees
Labels
priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@jrummell-chromium
Copy link
Contributor

With EME Logger enabled, on an internal site, I get the following error and the page fails to load properly. Without EME Logger enabled the page loads properly.

alerts.js:formatted:1 Uncaught SyntaxError: Identifier 'options' has already been declared (at alerts.js:formatted:1:1)

eme-trace-config.js does define const options = ..., so I assume that alerts.js does the same. The page should probably be changed to use unique names that are unlikely to collide with other pages, or wrap the variables inside a function.

(I also noticed that there are some global functions like byteToHexString. I wonder if they should be renamed (or placed inside a class)?)

@joeyparrish
Copy link
Member

Perhaps instead, the scripts that are injected into the page should have a scope wrapper:

(() => {
  const whateverIWantToCallIt = 'This will not leak';
})();

@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Apr 28, 2022
@github-actions github-actions bot added this to the Backlog milestone Apr 28, 2022
@joeyparrish joeyparrish changed the title Use unique names for global variables Pollution in global scope May 4, 2022
@joeyparrish joeyparrish added the priority: P1 Big impact or workaround impractical; resolve before feature release label May 4, 2022
@joeyparrish
Copy link
Member

See #45 for an example of a public site that breaks because of this

@matthuisman
Copy link

Fyi. This has been issue for quite a few years. I only just thought of disabling extensions to try to fix :)

@joeyparrish
Copy link
Member

@matthuisman, thanks for letting us know. We will work on an update to fix it.

joeyparrish pushed a commit that referenced this issue Jan 10, 2023
To avoid name collisions on sites due to common names, e.g. Uncaught
SyntaxError: Identifier 'options' has already been declared

This also updates to use version 1.0.2 of trace-anything.

Changes have been formatted with Chromium's JavaScript formatter.

Fixes #44
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Mar 11, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
3 participants