`/developer/url` definition should have `format: url` in JSON schema
Categories
(WebExtensions :: General, defect, P3)
Tracking
(firefox96 fixed)
Tracking | Status | |
---|---|---|
firefox96 | --- | fixed |
People
(Reporter: willdurand, Assigned: willdurand)
References
Details
(Whiteboard: [addons-jira])
Attachments
(1 file)
We discovered this bug as part of an addons-linter review: https://github.com/mozilla/addons-linter/pull/4002.
/developer/url
, introduced in Bug 1282977, overrides homepage_url
when it is present according to MDN. Given that homepage_url
has format: url
in its definition (added in bug 1244805), /developer/url
should definitely have that too.
As noted in the GitHub issue by :rpl, the result of not having format: url
is that setting an invalid url in developer.url
is ignored, and it does silently override a valid homepage url if one was specified, and the invalid url ends up in about:addons
.
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 1•2 years ago
|
||
This is likely a breaking change because it is going to reject some
invalid URLs: I am not too sure about how we should handle this. Values
other than URLs weren't expected, though, so I don't know.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 2•2 years ago
|
||
If there are a significant number of existing extensions that use a non-URL in this key, then we need to add "onError": "warn"
to the scheme to prevent them from breaking. Or just not adding a format validation.
Assignee | ||
Comment 3•2 years ago
|
||
:TheOne would you be able to run a code-search to know how many add-ons use developer.url
in their manifest? (not taking the value into account as a first step)
Comment 4•2 years ago
|
||
This is a little tricky to match precisely just using regular expressions/yara rules. Will suggested to start with searching for /"developer"/
, which yielded 15,000 hits for 4,000 add-ons. Let me know if you have ideas for a more precise pattern, I'm happy to run another search.
Comment 5•2 years ago
|
||
Is it possible to re-run the query to find all values that are in use? The full set can be reduced (later) to figure out how many invalid URLs are used.
We'd like to know that in order to know
- Whether we can need to treat an invalid value as a warning (comment 2).
- Whether we can make the linter stricter, per https://github.com/mozilla/addons-linter/issues/4038#issuecomment-966362648
Assignee | ||
Comment 6•2 years ago
|
||
(In reply to Rob Wu [:robwu] from comment #5)
Is it possible to re-run the query to find all values that are in use? The full set can be reduced (later) to figure out how many invalid URLs are used.
We'd like to know that in order to know
- Whether we can need to treat an invalid value as a warning (comment 2).
- Whether we can make the linter stricter, per https://github.com/mozilla/addons-linter/issues/4038#issuecomment-966362648
We'll definitely want to do that but finding more accurate data than what :TheOne already provided is going to take some time. We decided to use onError: warn
in the meantime.
Pushed by wdurand@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1b2c9b6344cc Add '"format": "url"' to 'developer.url' definition in JSON schema. r=rpl
Comment 8•2 years ago
|
||
bugherder |
Description
•