1470764 - Implement images.theme_ntp_background, properties.ntp_background_{alignment,repeat}

Status: NEW

(WebExtensions :: Themes, enhancement, P3)

Comment 1

[David Durst [:ddurst]]

NI for prioritization

Comment 2

[Tim Nguyen :ntim]

Attachment: Bug 1470764 - Activity stream changes.

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

Comment 3

[Tim Nguyen :ntim]

Attachment: Bug 1470764 - Implement images.theme_ntp_background, properties.ntp_background_{alignment,repeat}.

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

Comment 4

[Code Review Bot [:reviewbot]]

Comment on attachment 8990504 [details]
Bug 1470764 - Implement images.theme_ntp_background, properties.ntp_background_{alignment,repeat}.

https://reviewboard.mozilla.org/r/255574/#review262370


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/base/content/contentTheme.js:39
(Diff revision 1)
>    }],
> +  ["--newtab-background-image", {
> +    lwtProperty: "ntp_background_image",
> +    processValue(value, element) {
> +      if (!value) {
> +        return null; 

Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

Comment 5

[jingyu9575]

Hello,
Will this allow dynamic NTP background images? Looks like the type of `theme_ntp_background` is `ExtensionURL`, does it mean I can only use images packaged in the extension? Other properties seems to accept data URLs as well.
Thank you.

Comment 6

[Tim Nguyen :ntim]

(In reply to jingyu9575 from comment #5)
> Hello,
> Will this allow dynamic NTP background images? Looks like the type of
> `theme_ntp_background` is `ExtensionURL`, does it mean I can only use images
> packaged in the extension? Other properties seems to accept data URLs as
> well.
> Thank you.

Yes, this is for NTP background images. Dynamic theming will work, but data URLs won't work, simply for performance reasons (cross process communication is heavy).

Comment 7

[jingyu9575]

Thanks for the answer. 
I was thinking about using images not bundled in the extension, e.g. downloading new images daily from free wallpaer sites, or letting users upload their own images, and using them as the background. Without data URLs, how can I pass the non-bundled images to this API?

Comment 8

[Tim Nguyen :ntim]

In case someone wants to take over this patch, what's left to do is adding tests, and also fixing the fact that web_accessible_resources is required for the images to work.