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

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Description

[krupa raj [:krupa--use this to needinfo]]

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.

Comment 1

[Mike de Boer [:mikedeboer]]

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!

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

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"

Comment 3

[Matthew Wein [:mattw]]

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 4

[Matthew Wein [:mattw]]

Attachment: Bug 1339131 - Provide warnings instead of throwing errors for unrecognized properties

Review commit: https://reviewboard.mozilla.org/r/117954/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/117954/

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

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!

Comment 6

[Mike de Boer [:mikedeboer]]

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.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb9bf4230afe
Provide warnings instead of throwing errors for unrecognized properties r=jaws,mikedeboer

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/eb9bf4230afe