-
Notifications
You must be signed in to change notification settings - Fork 14
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
Update manifest.json with browser-specific permissions #529
Conversation
1284ab2
to
a1f8c8b
Compare
bbf7707
to
a1f8c8b
Compare
a1f8c8b
to
fdd9946
Compare
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.
Self-review
config-permissions.sh
Outdated
if [[ "$OSTYPE" == "darwin"* ]]; then | ||
sed -i '' "s|menus|contextMenus|g" src/manifest.json | ||
else | ||
sed -i "s|menus|contextMenus|g" src/manifest.json | ||
fi |
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.
Note this doesn't take an argument like the configure-domain.sh
script.
It's straight menus
--> contextMenus
"build:dev": "npm run build:clean && npm run config:dev && npm run build", | ||
"build:stage": "npm run build:clean && npm run config:stage && npm run build", | ||
"build:prod": "npm run build:clean && npm run config:prod && npm run build", |
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.
Context: Removed the package:chrome
script as this build script will no longer work for Chrome (without the extra script)
@@ -26,8 +30,7 @@ | |||
"web-ext-run:firefox": "npm run web-ext-run -- --target firefox-desktop", | |||
"web-ext-run:firefox-mv3": "npm run web-ext-run -- --firefox-preview --pref=extensions.eventPages.enabled=true --verbose", | |||
"web-ext-run:android": "npm run web-ext-run -- --target firefox-android --adb-device $npm_config_device --firefox-apk org.mozilla.fenix", | |||
"web-ext-run:chrome": "npm run web-ext-run -- --target chromium", | |||
"web-ext-run:desktop": "npm run web-ext-run -- --target chromium --target firefox-desktop" |
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.
Removed this npm script as it will the add-on can no longer work simultaneously.
fdd9946
to
6134b51
Compare
@@ -8,11 +8,16 @@ | |||
"web-ext": "^7.6.2" | |||
}, | |||
"scripts": { | |||
"build:dirty": "web-ext build -s src/", |
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.
This process moved the reset step (build:clean
) to the END of the Chrome-build script
package.json
Outdated
"build:chrome-dev": "npm run build:clean && npm run config:dev && npm run config:chrome && npm run build:dirty && npm run package:chrome && npm run build:clean", | ||
"build:chrome-stage": "npm run build:clean && npm run config:stage && npm run config:chrome && npm run build:dirty && npm run package:chrome && npm run build:clean", | ||
"build:chrome-prod": "npm run build:clean && npm run config:prod && npm run config:chrome && npm run build:dirty && npm run package:chrome && npm run build:clean", | ||
"config:chrome": "./config-permissions.sh", |
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.
suggestion (non-blocking): re-name this script to config-chrome.sh
?
Co-authored-by: luke crouch <[email protected]>
eaa1819
to
b38e594
Compare
Summary
Our Chrome submission of the add-on is being rejected because we have a Firefox-specific permission listed in the manifest file:
menus
Previously, this permission was ignored. It is now grounds for rejection: https://developer.chrome.com/docs/webstore/troubleshooting/#does-not-work
Jira: https://mozilla-hub.atlassian.net/browse/MPP-3466
TODO:
Update Chrome release steps in internal documentationWon't update until this PR is mergedStarting now, this project will only update the manifest (for Chrome) during build processes. This means without running the proper script, the add-on will not work for Chrome.
Testing
Confirm defaults work for Firefox
npm run dev
Confirm you can work on Chrome locally
npm run web-ext-run:chrome
Confirm you can build the add-on locally for a targeted platform/stage environment:
Note
You can also test by examining the output manifest.json file and confirming that Firefox builds have the
menus
permission while the Chrome builds havecontextMenus
.For Firefox/Stage
npm run build:stage
web-ext-artifacts
for a Firefox build (Test by installing on Nightly) pointed at stageFor Chrome/Prod
npm run build:chrome-prod
web-ext-artifacts
for a Chrome build (Test by installing on Chrome) pointed at prod