-
-
Notifications
You must be signed in to change notification settings - Fork 440
fix(manifest): ensure manifest author is string, support object with email #1792
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): ensure manifest author is string, support object with email #1792
Conversation
… email Convert author field in manifest to string if it is an object, using the email property. Add test to verify author object is correctly converted to string in the generated manifest. Prevents invalid manifest format in Firefox and other browsers.
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No browser other than Firefox needs the
I think these codes should be firefox only. |
|
Yes, before making the revisions, I consulted the documentation: But perhaps this way of handling it can avoid reporting errors on Firefox |
|
Does this also apply to stores like Opera, Edge Addons? |
@wxt-dev/analytics
@wxt-dev/auto-icons
@wxt-dev/browser
@wxt-dev/i18n
@wxt-dev/module-react
@wxt-dev/module-solid
@wxt-dev/module-svelte
@wxt-dev/module-vue
@wxt-dev/runner
@wxt-dev/storage
@wxt-dev/unocss
@wxt-dev/webextension-polyfill
wxt
commit: |
|
Hey @hellozzm I added some more tests and added a log as from my research as well no one uses the object version of this anymore and only Gecko based browsers use the plain string for display purposes. Could you review what I changed and give your feedback on it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aklinker1 would like your approval on this change, I don't consider it a breaking change since all browsers don't use it except gecko and we do migration but even then it's not correctly set I think. So I'd like you to merge this
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1792 +/- ##
==========================================
+ Coverage 81.04% 81.17% +0.12%
==========================================
Files 130 130
Lines 6649 6655 +6
Branches 1091 1093 +2
==========================================
+ Hits 5389 5402 +13
+ Misses 1249 1242 -7
Partials 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I’ve reviewed your changes—your test cases are much more comprehensive than mine. Overall, everything looks good now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert this file, it will be overwritten next time the @types/chrome package changes. Make the change to that package instead of here.
|
See #1770 (comment) |

Overview
Convert author field in manifest to string if it is an object, using the email property. Add test to verify author object is correctly converted to string in the generated manifest. Prevents invalid manifest format in Firefox and other browsers.
Manual Testing
Related Issue
This PR closes #1770