Don't error out on unknown properties in the theme object

RESOLVED FIXED in Firefox 55

Status

WebExtensions
General
RESOLVED FIXED
a year ago
6 days ago

People

(Reporter: krupa, Assigned: mattw)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [themes, triaged])

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Looks like "background_tab" and "background_tab_inactive" properties are not in the schema ( https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/schemas/theme.json) because of which Firefox throws an error if they are listed in the manifest.json

We should not error out on unknown properties in the theme object.
See Also: → bug 1244474
I think bug 1244474 should already cover things here! This also fits in nicely with us not needing to implement stubs for unsupported Google Chrome theme properties. Nice!
Blocks: 1330335
No longer blocks: 1330328

Updated

a year ago
Whiteboard: [themes, triaged]
This is still broken. I added a "background_tab_jared" colors property and got the following error in about:debugging,

Log.jsm:748
1488905516732	addons.webextension.<unknown>	ERROR	Loading extension 'null': Reading manifest: Error processing theme.colors: Unexpected property "background_tab_jared"

controls.js:63:11
Error: Extension is invalid
Stack trace:
loadManifestFromWebManifest<@resource://gre/modules/addons/XPIProvider.jsm:975:11
TaskImpl_run@resource://gre/modules/Task.jsm:319:42
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:742:11

This is the error I get when loading it in about:addons:

Log.jsm:748
1488905679777	addons.webextension.<unknown>	ERROR	Loading extension 'null': Reading manifest: Error processing theme.colors: Unexpected property "background_tab_jared"
(Assignee)

Comment 3

a year ago
I believe we just need to add an `additionalProperties` section which references `UnrecognizedProperty` to each of the theme sections to fix this: http://searchfox.org/mozilla-central/source/browser/components/extensions/schemas/browser_action.json#14. I think a manifest test would be good to add also to make sure this works.
Comment hidden (mozreview-request)
Comment on attachment 8844571 [details]
Bug 1339131 - Provide warnings instead of throwing errors for unrecognized properties

https://reviewboard.mozilla.org/r/117954/#review119700

Awesome, thanks!
Attachment #8844571 - Flags: review?(jaws) → review+

Comment 6

a year ago
mozreview-review
Comment on attachment 8844571 [details]
Bug 1339131 - Provide warnings instead of throwing errors for unrecognized properties

https://reviewboard.mozilla.org/r/117954/#review120034

BOOM! Nice one.
Attachment #8844571 - Flags: review?(mdeboer) → review+

Comment 7

a year ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eb9bf4230afe
Provide warnings instead of throwing errors for unrecognized properties r=jaws,mikedeboer

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eb9bf4230afe
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

6 days ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.