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

request: manifest error handling for unknown properties #14

Closed
therealglazou opened this issue Jun 18, 2021 · 16 comments
Closed

request: manifest error handling for unknown properties #14

therealglazou opened this issue Jun 18, 2021 · 16 comments
Labels
inconsistency Inconsistent behavior across browsers spec clarification Needs clarification when specified supportive: chrome Supportive from Chrome supportive: firefox Supportive from Firefox supportive: safari Supportive from Safari

Comments

@therealglazou
Copy link

An issue hit by all WebExtension vendors is related to manifest properties unrecognized by a given browser. A good example is browser_specific_settings that Firefox relies on but Chrome does not accept at all, triggering a manifest error on install. This forces all of us to have two different manifests or dynamically generate them or even have two different builds.

I think we could/should specify two things wrt manifest:

  • error handling; what should happen when an error happens in the manifest? Could some existing errors become warnings if it's a value error and a default exists?
  • author-defined values à la "x-" reachable through chrome.runtime.getManifest()
@xeenon
Copy link
Collaborator

xeenon commented Jun 18, 2021

Safari also supports browser_specific_settings with a safari sub-key.

@mugcake
Copy link

mugcake commented Jun 22, 2021

More details about the browser_specific_settings object.

@rubenwardy
Copy link

rubenwardy commented Aug 6, 2021

The following is copied from a bugzilla.mozilla.org issue that I made requesting the ability to set general manifest settings for particular browsers.

Steps to reproduce:

One of the benefits of webextensions is being able to create a cross-platform browser extension from the same code base. Unfortunately, different browsers behave differently so there needs to be ways to account for the differences in the code and the configuration.

browser_specific_settings allows setting specific settings for each browser. I'd like the ability to override any property from the root of the manifest in a similar way.

For example, on Chrome you're not allowed to set both the new tab and homepage. However, on Firefox you are, and the homepage is used to control the new window page.

Example code:

{
	"manifest_version": 2,
	"name": "Renewed Tab",
	"chrome_url_overrides" : {
		"newtab": "app/webext.html"
	},

	"browser_specific_settings": {
		"gecko": {
			"chrome_settings_overrides": {
				"homepage": "app/webext.html"
			}
		}
	}
} 

Actual results:

The example code resulted in an error, as chrome_settings_overrides wasn't an expected field

Expected results:

It should have overridden chrome_settings_overrides on Firefox (=gecko) only

@xeenon xeenon added inconsistency Inconsistent behavior across browsers spec clarification Needs clarification when specified future topics labels Sep 2, 2021
@dotproto
Copy link
Member

dotproto commented Sep 2, 2021

@therealglazou, could you expand on the use case for author-defined values? My initial reaction is that retrieving other JSON data seems like it would be better served by fetching a separate JSON file.

@carlosjeurissen
Copy link
Contributor

carlosjeurissen commented Sep 3, 2021

@rubenwardy

If such syntax is introduced, it should receive it's own property within browser_specific_settings or a new property under manifest to prevent any conflicts with current / future properties under browser_specific_settings.

So we could go for something like this:
"browser_specific_settings": {
"gecko": {
"overrides": {
"name": "Firefox specific name"
}
}
}

Or do something like this:
"browser_specific_overrides": {
"gecko": {
"name": "Firefox specific name"
}
}

As an extension developer myself. I do see the use case in this. In the past I wanted to introduce a different name for an extension for firefox. Then there is sidebar_action which can't live next to browser_action in naver whale. Making is impossible to do this for Firefox and Opera. Tho we probably want to limit this ability as much as possible to prevent issues we faced with user-agent strings and vendor prefixes.

@therealglazou
Copy link
Author

@therealglazou, could you expand on the use case for author-defined values? My initial reaction is that retrieving other JSON data seems like it would be better served by fetching a separate JSON file.

Several reasons:

  • good frameworks are extensible frameworks
  • if we allow user-defined properties, then our error handling rules on property names must become "accept all, do nothing on the ones the browser ignores" which is clearly the best in the Web world
  • Excerpt from developer.chrome.com: « Every extension has a JSON-formatted manifest file, named manifest.json, that provides important information ». Does the manifest allow the vendor to add licensing information (that is important, right?) through a URL? No. Could the vendor add it through a user-defined manifest property? Sure.

@carlosjeurissen
Copy link
Contributor

An issue has been opened on ignoring some of these for Chromium:
https://bugs.chromium.org/p/chromium/issues/detail?id=1246358

@carlosjeurissen
Copy link
Contributor

@therealglazou Allowing custom author-defined values can conflict with future additions to the extension APIs. In addition it makes it harder to find issues in the extension manifest. As mentioned, additional info can easily go into a separate json file you fetch in the code. or using a security.txt, readme.md, license.txt or similar.

@carlosjeurissen
Copy link
Contributor

Lets not forget about custom / unknown properties further down the manifest.json three. See:
Issue on the chromium-extensions group about custom properties of content_scripts entries.

@carlosjeurissen
Copy link
Contributor

An issue related to how to handle manifest unknown keys / unsupported keys on Firefox can be found here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1723156

@oliverdunk
Copy link
Member

@dotproto, do you remember if there were any updates from the Chromium side here? I feel like it came up in a more recent meeting but I don't see that in the notes.

@dotproto dotproto added the follow-up: chrome Needs a response from a Chrome representative label Jul 7, 2022
@Rob--W Rob--W added supportive: safari Supportive from Safari supportive: firefox Supportive from Firefox labels Feb 24, 2023
@Rob--W
Copy link
Member

Rob--W commented Feb 24, 2023

This topic has come up a few times, and the consensus is to not fail hard unless really necessary.

Unrecognized top-level keys already are already "ignored", as in not triggering hard failures that prevent the installation of an extension across browsers. Chrome and Firefox both print warnings about unrecognized properties as a debugging aid.

Safari's implementation only refuses the manifest in really dire situations, such as the manifest being invalid JSON.

Firefox's implementation currently accepts unrecognized manifest keys in many manifest fields, but there are also plenty that do not, because in our implementation the permissive behavior is opt-in. Because of this, sometimes an unrecognized manifest key causes a hard installation failure. We are willing to change this for manifest.json validation.

Chrome's manifest validation is customized for each manifest key. The default behavior is to ignore unknown properties, with some exceptions for cases where Chrome is concerned about the presence of an unsupported key.

@hanguokai
Copy link
Member

I think the root of this problem is that the current extension manifest file only has implementations but no standard specification. Some properties or values cannot be simply ignored, and some can. What are the validation rules? This requires establishing a specification first. Then browsers adjusts their behavior according to the specification.

@oliverdunk oliverdunk added supportive: chrome Supportive from Chrome and removed follow-up: chrome Needs a response from a Chrome representative labels Mar 23, 2023
@oliverdunk
Copy link
Member

I think we'll want to have more discussion as we talk about specific changes more, but in general Chrome is supportive of this.

@Rob--W
Copy link
Member

Rob--W commented May 4, 2023

Firefox's implementation currently accepts unrecognized manifest keys in many manifest fields, but there are also plenty that do not, because in our implementation the permissive behavior is opt-in. Because of this, sometimes an unrecognized manifest key causes a hard installation failure. We are willing to change this for manifest.json validation.

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1831417 to track the ability to treat unrecognized manifest keys as warnings instead of errors, by default.

@oliverdunk
Copy link
Member

We discussed this at our in person meeting in San Diego. All browsers are generally supportive of not having hard errors in most cases, and we are making changes in that direction, e.g Chrome's change to handling of background manifest keys.

Changes to getManifest() are tracked here: #400

Closing this since there isn't a specific action item but hope we can continue making progress here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inconsistency Inconsistent behavior across browsers spec clarification Needs clarification when specified supportive: chrome Supportive from Chrome supportive: firefox Supportive from Firefox supportive: safari Supportive from Safari
Projects
None yet
Development

No branches or pull requests

9 participants