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

Consider introducing a new error code for optional-only permissions #5554

Open
rpl opened this issue Jan 21, 2025 · 2 comments
Open

Consider introducing a new error code for optional-only permissions #5554

rpl opened this issue Jan 21, 2025 · 2 comments

Comments

@rpl
Copy link
Member

rpl commented Jan 21, 2025

As a followup to #5551 we want to consider introducing a new error code to be associated to optional-only permissions being requested as non-optional.

Reasons for introducing a new error code would be:

  • make it clear when a permission is invalid because non supported by Firefox vs. when it is invalid because it can only requested as optional
  • point developers to a doc page describing in a bit more details how developers should use optional-only permissions (and how that differs from other optional permissions).
@Rob--W
Copy link
Member

Rob--W commented Jan 30, 2025

We have a stub for MDN documentation, which we should link that from the dedicated message: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/optional_permissions#optional-only_permissions

@Rob--W
Copy link
Member

Rob--W commented Feb 27, 2025

The current warning is Invalid permissions "${permName}", associated with the permissions manifest key.

This message is confusing. It would be nice if the message is more specific, e.g. Permission "${permName}" is only supported in optional_permissions.

In Chrome, userScripts permission has to be in permissions (or optional_permissions), in Firefox it can only be in optional_permissions. An extension that would like to have a cross-browser compatible extension would put the permission in both permissions and optional_permissions. In that case, we should ideally not warn.
(we have a similar situation where we should warn, optional_permissions vs optional_host_permissions, see #5329)

Relevant unit test (from PR #5551) is at

describe('optional only permissions', () => {
const OPTIONAL_ONLY_PERMISSIONS = ['userScripts', 'trialML'];
it.each(OPTIONAL_ONLY_PERMISSIONS)(
'should warn on optional-only permission "%s" requested as non-optional',
(permName) => {
const linter = new Linter({ _: ['bar'] });
const json = validManifestJSON({
permissions: ['tabs', permName],
browser_specific_settings: { gecko: { strict_max_version: '135.0' } },
});
const manifestJSONParser = new ManifestJSONParser(
json,
linter.collector
);
expect(manifestJSONParser.collector.warnings).toEqual(
expect.arrayContaining([
expect.objectContaining({
code: 'MANIFEST_PERMISSIONS',
instancePath: '/permissions/1',
message: expect.stringMatching(
new RegExp(`Invalid permissions "${permName}"`)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants