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

fix manifest #2476

Merged
merged 1 commit into from
Oct 12, 2023
Merged

fix manifest #2476

merged 1 commit into from
Oct 12, 2023

Conversation

patricklx
Copy link
Collaborator

@patricklx patricklx commented Oct 10, 2023

fixes #1158

@patricklx patricklx marked this pull request as ready for review October 10, 2023 11:50
@RobbieTheWagner
Copy link
Member

@patricklx was matches all we were missing then? What exactly does this do?

@patricklx
Copy link
Collaborator Author

i'm not sure 😅
thats from https://developer.chrome.com/docs/extensions/migrating/manifest/#update-wa-resources
and the error reported when loading the extension locally into chrome

@patricklx patricklx marked this pull request as draft October 10, 2023 11:57
@RobbieTheWagner
Copy link
Member

i'm not sure 😅 thats from https://developer.chrome.com/docs/extensions/migrating/manifest/#update-wa-resources and the error reported when loading the extension locally into chrome

IIRC things worked locally for me, even with manifest v3. You were seeing an error? Does this resolve that error?

@patricklx
Copy link
Collaborator Author

yes, it loads the extension now, but it fails to load on any page...
some csp issue

@patricklx
Copy link
Collaborator Author

I understand now. adding inline scripts directly is not allowed any more. But its possible to load scripts that are specified in the allowed resourced inside the manifest.
I will add fixes for that as well here

@@ -90,7 +90,7 @@
if (!injected) {
// cannot use eval here, as the context is limited to the content script-
const elem = document.createElement('script') ;
elem.textContent = message.value;
elem.src = chrome.runtime.getURL('panes-3-16-0/ember_debug.js');
Copy link
Member

Choose a reason for hiding this comment

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

Will specifically loading 3-16-0 here break our check where we switch to different versions for older Ember?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, just testing now. I will change it later

@patricklx patricklx marked this pull request as ready for review October 10, 2023 13:42
@patricklx patricklx marked this pull request as draft October 10, 2023 13:42
@patricklx
Copy link
Collaborator Author

I think thizis ready.
Let me squash all commits

@patricklx patricklx force-pushed the patch-4 branch 2 times, most recently from 0b34c6b to 3c49cfe Compare October 10, 2023 14:53
@patricklx patricklx marked this pull request as ready for review October 10, 2023 14:55
@patricklx
Copy link
Collaborator Author

Squashed and ready

@patricklx patricklx force-pushed the patch-4 branch 2 times, most recently from 7d90cd5 to d41b662 Compare October 10, 2023 15:46
@patricklx
Copy link
Collaborator Author

this might also fix #1158

@RobbieTheWagner
Copy link
Member

@patricklx have you checked to make sure the older versions still boot up for Ember < 3.16?

Copy link
Member

@RobbieTheWagner RobbieTheWagner left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thank you so much for your help 🎉. I just want to make sure we test these changes against some older Ember versions, to make sure loading the old versions works still.

@patricklx
Copy link
Collaborator Author

patricklx commented Oct 12, 2023

finally found a website...
http://altrim.github.io/ember-cli-tooltipster/ is using Ember 2.10.1
it loads correctly, uses older ember_debug and ember-inspector.
it loaded first panes-3-16/ember_debug.js, then it detects the version and sends version-mismatch which triggers the loading of older version

@RobbieTheWagner RobbieTheWagner merged commit 6821f55 into emberjs:main Oct 12, 2023
15 checks passed
@patricklx patricklx deleted the patch-4 branch October 12, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants