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

Support $schema in manifest.json #507

Open
lucasrmendonca opened this issue Dec 14, 2023 · 25 comments
Open

Support $schema in manifest.json #507

lucasrmendonca opened this issue Dec 14, 2023 · 25 comments
Labels
implemented: chrome Implemented in Chrome proposal Proposal for a change or new feature supportive: firefox Supportive from Firefox supportive: safari Supportive from Safari

Comments

@lucasrmendonca
Copy link

lucasrmendonca commented Dec 14, 2023

Is it possible to allow the $schema key in manifest.json as a valid optional key?

Currently, some editors such as VSCode allow for type-checking json files by using the $schema key. This is particularly useful for beginners and for folks migrating their manifests from version 2.0 to 3.0 since it gives out squiggly lines if you mistype something in your json file. Tools such as Cspell and Knip even provide the $schema key by default in their docs.

When I tried using "$schema": "http://json.schemastore.org/chrome-manifest" in manifest.json, although type-checking worked fine, when loading the extension to the browser, an Error appears: Unrecognized manifest key '$schema'.

This message doesn't stop type-checking from working, but it creates a persistent "Errors" button at chrome://extensions page for our users, which isn't desirable. If possible, allowing it to be a valid optional key would be great.

@xeenon
Copy link
Collaborator

xeenon commented Dec 14, 2023

This is likely a Chrome only issue, since Safari allows it and I think Firefox is lax about unknown manifest keys as well.

@xeenon xeenon added proposal Proposal for a change or new feature and removed needs-triage labels Dec 14, 2023
@fregante
Copy link

I think this key is straight out rejected by the store actually. For the first time I tried it last week and the store rejected my first zip upload. I removed the key and it worked.

@dotproto
Copy link
Member

dotproto commented Jan 4, 2024

@fregante, which store are you referring to?

@fregante
Copy link

fregante commented Jan 4, 2024

Chrome Web Store

@carlosjeurissen
Copy link
Contributor

@fregante Just tested in Chrome. Loading an extension with a $schema property seems to work fine. So it seems a Chrome Web Store only issue.

@lucasrmendonca
Copy link
Author

lucasrmendonca commented Jan 4, 2024 via email

@Rob--W
Copy link
Member

Rob--W commented Jan 4, 2024

I'm still getting the warning here. Which Chrome version are you using? I'm using Chrome Version 120.0.6099.199 (Official Build) (64-bit) on Ubuntu: "*⚠️ Unrecognized manifest key '$schema'.

A warning is not a fatal error.

@lucasrmendonca
Copy link
Author

A warning is not a fatal error.

I think we're discussing two different issues here, this thread is originally about removing the warning about this manifest key that appears when clicking on the "Errors" button in chrome://extensions page.

@fregante mentioned another issue about Chrome Web Store rejection, maybe we could create a new thread for that to avoid confusion?

@fregante
Copy link

fregante commented Jan 5, 2024

I think the issue is the same: officially recognizing and allowing the $schema property. Having the browser not warn but then the store reject the extension is worse. The store must accept the property before the browser removes the warning.

@lucasrmendonca
Copy link
Author

lucasrmendonca commented Jan 5, 2024

That makes sense @fregante. However, it seems like the meeting attendees didn't understand it that way. Quoting the meeting notes:

[@patrickkettner] The main request is to stop treating $schema as a hard error in Chrome. Other browsers don’t fail here.
[@oliverdunk ] I think that we are supportive on Chrome’s end - we are moving towards treating unrecognized/unsupported keys as non-fatal warnings instead of errors.
[@carlosjeurissen ] Currently you can load an extension just fine in Chrome with the $schema property. The issue seems to be present in the Chrome Web Store only.

That isn't the main request. It's treating it as a recognized/supported key instead (both in the browser by removing the warning and in the Chrome Web Store by not letting it be rejected)

@patrickkettner
Copy link
Contributor

@lucasrmendonca thanks for checking the notes! We are saying the same thing, though.

As this is the WECG, there is nothing to do with the Chrome Web Store. So the relevant request here is for it to be recognized in Chrome. This means removing the hard error.

@lucasrmendonca
Copy link
Author

lucasrmendonca commented Jan 5, 2024

@patrickkettner I see, maybe the confusion is related to calling it a hard error instead of a non-fatal warning?

Judging by @carlosjeurissen and @Rob--W replies, it seems like they were expecting the extension to not work properly in Chrome when using the $schema key, which was never a problem

@patrickkettner
Copy link
Contributor

Apologies. There is confusion on verbiage. I was under the impression that when $schema was in the manifest file, it was a fatal error, not just a warning (in the confusingly named "errors" page). If that isn't the case then I believe there is nothing to do in the WECG, unless you can think of a reason the $schema information could be used in browsers that would make sense to standardize (other than "let it exist").

If not, then we can close this and I will open a bug with the Chrome Web Store team internally to allow it.

@lucasrmendonca
Copy link
Author

unless you can think of a reason the $schema information could be used in browsers that would make sense to standardize (other than "let it exist").

It would be great for the WECG to standardize it since having manifest.json type-checking:

  • Helps with on-boarding new extension developers to the platform since it shows squiggly lines when manifest.json values are in the wrong format
  • Helps with migrating extensions from Manifest 2.0 to 3.0 (useful with MV2 EOL coming this year)
  • Is already being encouraged in .json configuration files in some other tools (see these docs for Cspell and Knip for example)

If the WECG decides it's still not useful enough it to become a standard, I'm happy with at least not discouraging it by removing the warning in Chrome...

@patrickkettner
Copy link
Contributor

What is the specific request? What is the thing you want produced by the WECG as a result of this bug

@lucasrmendonca
Copy link
Author

Removing the warning for this key in Chrome and opening a bug with the Chrome Web Store team internally to allow it

@fregante
Copy link

fregante commented Jan 5, 2024

WECG could formally recognize and allow the field, so that Chrome and other implementor don't risk rejecting it.

If WECG rejects that, then it's just a "bug" that should be reported to Chrome.

@patrickkettner
Copy link
Contributor

@lucasrmendonca the only member of the WECG that shows a warning is Chrome. The WECG doesn't have a specification for the manifest file at the moment, so beyond the agreement to what browsers are already doing, there doesn't seem to be a conversation to be had.

As for the Chrome Web Store bug, it is outside of the scope of the WECG, so it wouldn't be related to wether or not it remains open

@fregante what do you mean by formally recognize? What does that actually, concretely, mean? To be clear, I am a representative of Chrome, as is Oliver. We aren't rejecting it. The WECG isn't rejecting this issue as of now, but it isn't clear what is being requested.

@lucasrmendonca
Copy link
Author

lucasrmendonca commented Jan 5, 2024

@patrickkettner I thought the specification for the manifest file and its allowed keys was part of the WECG scope. It seems like something that should be agreed across all browsers instead of dealt on a per-browser basis (let me know if I'm wrong here). Are you suggesting we move this discussion to somewhere else that involves only Chrome since it's the only browser that displays the warning for $schema ?

@fregante
Copy link

fregante commented Jan 5, 2024

what do you mean by formally recognize? What does that actually, concretely, mean?

I'm not sure how detailed you want my request to be. Do "Web Extensions" have a specification? Is it this one? If so, WECG should mention and define $schema. I don't know how else to word this request.

Whether other browsers already allow it is irrelevant here; they could, at any time, stop allowing it, because nobody cared to include it in the specification. And if Chrome doesn't allow it, why should they?

@Rob--W
Copy link
Member

Rob--W commented Jan 5, 2024

what do you mean by formally recognize? What does that actually, concretely, mean?

I'm not sure how detailed you want my request to be. Do "Web Extensions" have a specification? Is it this one?

The correct URL is https://w3c.github.io/webextensions/specification/ ("browserext" is not relevant and obsolete).

If so, WECG should mention and define $schema. I don't know how else to word this request.

Whether other browsers already allow it is irrelevant here;

You have expressed a request to specify a new key in the manifest.json file, seemingly primarily motivated by the presence of an error message in Chrome. What other browsers do IS relevant, because as of now it looks like a Chrome-only issue. There is consensus among browser vendors to make sure that unrecognized keys should not prevent extensions from loading.

A key being unrecognized should not be a problem. If it was, we'd like to understand why, in order to work towards a resolution. After all, we recognize that unsupported keys exist, and that their presence in a manifest should not cause serious issues.

It would be helpful if you state more clearly why the presence of a non-fatal warning is an issue to you. There is already a request on Chrome's issue tracker about removing the warning for some unsupported keys, but with little progress and the why-question being unanswered, at https://crbug.com/1246358

@oliverdunk
Copy link
Member

In the meeting, I definitely thought the report was that this was a hard error. With that not being the case, I think the importance of this fix is a little diminished, but I still appreciate @lucasrmendonca's perspective that it would be nice to avoid warnings for some keys that developers may wish to use despite there being no special handling in the browser for them. I've left a comment on the bug to that effect which is likely the best place to continue this part of the discussion.

I tried reproducing the issue in the web store but wasn't able to. Can anyone still reproduce that? As @patrickkettner said one of us would be happy to raise that with the team if we can find any issues.

Finally, for standardisation, I think having a "these keys are not used but browsers should ignore them" section in the spec we would like to eventually write would be nice. The specification is so bare bones right now though that I'm not sure there is any action item to take today unless someone wants to try drafting that (and we would need to discuss as a group if we actually want to merge something like that).

@fregante
Copy link

fregante commented Jan 7, 2024

I tried reproducing the issue in the web store but wasn't able to

I tried uploading the extension I had issues with and I wasn't able to see the issue either. It's strange because my upload last month failed and it resolved after I dropped $schema. Might have been a coincidence. Sorry for the noise.

Just for reference, here's the successful upload with $schema: https://github.com/w3c/webextensions/assets/1402241/bd216da2-dc40-49d2-b8c4-fa48c112fb46

Finally, for standardisation, I think having a "these keys are not used but browsers should ignore them" section in the spec we would like to eventually write would be nice

💙 👍 That sounds perfect. Maybe they could be a list of "reserved" or "private" list of keys.

@Rob--W
Copy link
Member

Rob--W commented Sep 25, 2024

We discussed this in an in-person meeting at TPAC 2024 (#659).

Since $schema is standardized, there is consensus on special-casing them, to ignore them and not warn.

Related, I also raised the same issue about browser_specific_settings in Chrome, and Devlin is in favor of ignoring this one too. On the question of whether to include chrome in browser_specific_settings (#115), the position is neutral.

Oliver is starting a patch for $scheme and browser_specific_settings at https://issues.chromium.org/issues/40196501

@Rob--W Rob--W added supportive: chrome Supportive from Chrome supportive: safari Supportive from Safari labels Sep 25, 2024
@Rob--W Rob--W added the supportive: firefox Supportive from Firefox label Sep 25, 2024
aarongable pushed a commit to chromium/chromium that referenced this issue Sep 30, 2024
These keys are widely adopted and are understood to not have special
behavior in Chrome. Therefore, don’t generate manifest warnings if these
keys are specified.

This was agreed on in the WebExtensions Community Group:

w3c/webextensions#507

Bug: 40196501
Change-Id: Ia6d14031e6f4af6b8c7672a6f368685a2a9cdcbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5889684
Reviewed-by: Devlin Cronin <[email protected]>
Commit-Queue: Oliver Dunk <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1361955}
@oliverdunk
Copy link
Member

Starting in Chrome 131, the $schema and browser_specific_settings keys are ignored without a warning in Chrome: https://chromiumdash.appspot.com/commit/8ade57386c4c99c46a104ede72d6a4f5379448d4

You can see the schedule for rollout here.

@oliverdunk oliverdunk added implemented: chrome Implemented in Chrome and removed supportive: chrome Supportive from Chrome labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implemented: chrome Implemented in Chrome proposal Proposal for a change or new feature supportive: firefox Supportive from Firefox supportive: safari Supportive from Safari
Projects
None yet
Development

No branches or pull requests

8 participants