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

correctly load scripts in chrome & firefox #2501

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

patricklx
Copy link
Collaborator

@patricklx patricklx commented Nov 8, 2023

this does not work for firefox

Reading manifest: Warning processing content_scripts.0.world: An unexpected property was found in the WebExtension manifest.

Also has warnings about permissions

but firefox keeps the order right for script loading

it needs to run before everything else so that ember gets the debug config. If the property is set later, component tree will not work.
this issue only happens on page refresh, then browser uses cached data and immediately runs the scripts, without waiting for the new script that have been added by extensions. this does not happen in firefox, only chrome.
https://bugs.chromium.org/p/chromium/issues/detail?id=634381

fixes #2500

@patricklx patricklx marked this pull request as ready for review November 8, 2023 08:38
@patricklx patricklx changed the title merge ember env insert boot script as first child Nov 8, 2023
@patricklx patricklx marked this pull request as draft November 8, 2023 08:44
@patricklx patricklx changed the title insert boot script as first child run boot script in MAIN world Nov 8, 2023
@patricklx patricklx marked this pull request as ready for review November 8, 2023 11:16
@patricklx patricklx changed the title run boot script in MAIN world correctly load scripts in chrome & firefox Nov 8, 2023
@patricklx patricklx closed this Nov 8, 2023
@patricklx patricklx reopened this Nov 8, 2023
@@ -3,6 +3,7 @@
// TODO: make this conditional, only do it when requested.
// only inject boot script when on an HTML page
if (document.contentType === 'text/html') {
window.EmberENV = { _DEBUG_RENDER_TREE: true };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't we want to check for existence of window.EmberENV here too instead of setting it to our own object?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed here. This really absolutely runs before anything else

@@ -273,19 +273,31 @@ module.exports = function (defaults) {

replacementPattern = replacementPattern.concat(emberInspectorVersionPattern);

const firefoxBackgroundServiceReplacement = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for? Firefox should support manifest v3, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't... Not all of it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is available since manifest v3, but ignored in firefox, use wrappedJSObject for firefox
https://developer.chrome.com/docs/extensions/reference/scripting/#type-ExecutionWorld
@patricklx
Copy link
Collaborator Author

@RobbieTheWagner RobbieTheWagner merged commit 7d1208b into emberjs:main Nov 12, 2023
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inspector is stomping app's EmberENV
2 participants