Closed Bug 1432508 Opened 2 years ago Closed 2 years ago

Support default_locale in static theme manifests

Categories

(WebExtensions :: Themes, enhancement, P5)

enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

- default_locale
- update_url

should be accepted
I won't add update_url on second thought, since we don't support it for extensions either.
Summary: Accept more properties in static theme manifests → Relax WebExtension theme manifest parsing
I couldn't find anything else than default_locale
Summary: Relax WebExtension theme manifest parsing → Support default_locale in static theme manifests
Comment on attachment 8944837 [details]
Bug 1432508 - Support default_locale in static theme manifests.

https://reviewboard.mozilla.org/r/214994/#review220668

What's the scope here?  Is it just localizing manifest fields like name and description?  Whatever we're trying to support should have accompanying tests to prevent future regressions.
Attachment #8944837 - Flags: review?(aswan) → review-
Flags: needinfo?(ntim.bugs)
Component: WebExtensions: General → WebExtensions: Frontend
Priority: -- → P5
Component: WebExtensions: Frontend → WebExtensions: Themes
Flags: needinfo?(ntim.bugs)
Comment on attachment 8944837 [details]
Bug 1432508 - Support default_locale in static theme manifests.

https://reviewboard.mozilla.org/r/214994/#review244680

::: toolkit/components/extensions/schemas/i18n.json:10
(Diff revision 2)
>  [
>    {
>      "namespace": "manifest",
>      "types": [
>        {
> -        "$extend": "WebExtensionManifest",
> +        "$extend": "ManifestBase",

I'm not sure we want this.  Language packs also use ManifestBase and dictionaries eventually will too and it doesn't seem right for them.  There's not a great solution but for now lets just add it directly to ThemeManifest

::: toolkit/mozapps/extensions/test/xpcshell/test_webextension.js:137
(Diff revision 2)
>    addon.uninstall();
>  
>    await promiseRestartManager();
>  });
>  
> +// Test that default_locale works with WE themes

please move this test to test_webextension_theme.js
Attachment #8944837 - Flags: review?(aswan) → review-
Comment on attachment 8944837 [details]
Bug 1432508 - Support default_locale in static theme manifests.

https://reviewboard.mozilla.org/r/214994/#review246850

thanks
Attachment #8944837 - Flags: review?(aswan) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b74e2721b7f0
Support default_locale in static theme manifests. r=aswan
Also, browser chrome failures on /browser_ext_themes_dynamic_getCurrent.js

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=176517239&repo=autoland&lineNumber=4344
I'll take care of this.
Flags: needinfo?(aswan)
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a662fe1cc8ba
Support default_locale in static theme manifests. r=aswan
https://hg.mozilla.org/mozilla-central/rev/a662fe1cc8ba
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.