1311722 - The theme API should be able to support dynamic updates

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Description

[Matthew Wein [:mattw]]

The goal of this bug is to expose and test a function that can update a single property dynamically.

Comment 1

[Matthew Wein [:mattw]]

Attachment: Bug 1311722 - Add support for dynamic theme updates

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

Comment 2

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

Comment on attachment 8803416 [details]
Bug 1311722 - Add support for dynamic theme updates

https://reviewboard.mozilla.org/r/87694/#review86690

Cool, I like the update function and think it is a nice way for now to batch the updates of the theme so they all apply at once.

::: browser/components/extensions/test/browser/browser_ext_theme_abouthomebackground.js:5
(Diff revision 1)
> -const kBackground = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==";
> +const kBackground1 = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==";
> +const kBackground2 = "data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIxNiIgaGVpZ2h0PSIxNiIgdmlld0JveD0iMCAwIDE2IDE2Ij4KICA8cGF0aCBkPSJNOCwxMkwzLDcsNCw2bDQsNCw0LTQsMSwxWiIgZmlsbD0iIzZBNkE2QSIgLz4KPC9zdmc+Cg==";

Can you change this to an array of backgrounds?

const kBackgrounds = [
  "data:image/png; ...",
  "data:image/png; ..."
];

Comment 3

[Mike de Boer [:mikedeboer]]

Comment on attachment 8803416 [details]
Bug 1311722 - Add support for dynamic theme updates

https://reviewboard.mozilla.org/r/87694/#review86756

Just like I had hoped, nice! I don't see us winning much with using the fancy `class` syntax, but I must admit the legibility did improve somewhat ;)

Comment 4

[Matthew Wein [:mattw]]

Comment on attachment 8803416 [details]
Bug 1311722 - Add support for dynamic theme updates

https://reviewboard.mozilla.org/r/87694/#review86690

> Can you change this to an array of backgrounds?
> 
> const kBackgrounds = [
>   "data:image/png; ...",
>   "data:image/png; ..."
> ];

Done :)

Comment 5

[Matthew Wein [:mattw]]

Comment on attachment 8803416 [details]
Bug 1311722 - Add support for dynamic theme updates

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/87694/diff/1-2/

Comment 6

[Matthew Wein [:mattw]]

Comment on attachment 8803416 [details]
Bug 1311722 - Add support for dynamic theme updates

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/87694/diff/2-3/

Comment 7

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/projects/cedar/rev/217ff2dc022c72221ad8d7454803fde332c772c5
Bug 1311722 - Add support for dynamic theme updates r=jaws