1313325 - Add an extreme example to demo the Theme API capabilities

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Description

[Mike de Boer [:mikedeboer]]

Our meeting notes mention the following plan:

WebExtensions API extreme example
 * Whimsy rainbow - psychedelic? (js running that changes colors, disco browser)
 * if we did allow icon swaps we could show the menu icon being replaced by a hamburger?
 * lots of cats

But please leave more creative ideas in the comments!

Comment 1

[Mike de Boer [:mikedeboer]]

It is by no means necessary that I work alone on this bug - a collaborative effort is much appreciated ;-)

Comment 2

[Tim Nguyen :ntim]

WIP VivaldiFox port to WebExt: https://github.com/nt1m/vivaldi-fox/tree/webextension

I only plan to make page color extraction work for this prototype. I don't plan to implement other features the extension currently has (custom themes, settings, ...)

Meta tag color extraction works well (assuming bug 1314145 is fixed)
What's left is icon color extraction, and most cases should be covered.

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: My take on something extreme

The manifest shown in the test file is not showing the fun stuff. Please watch this video to get a 'feel' for it: https://vimeo.com/190708433

Please, let me know what you think! :P

Comment 4

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #3)
> Created attachment 8808598 [details] [diff] [review]
> My take on something extreme
> 
> The manifest shown in the test file is not showing the fun stuff. Please
> watch this video to get a 'feel' for it: https://vimeo.com/190708433
> 
> Please, let me know what you think! :P

Looks absolutely great! :D How did you get the sound ?

Comment 5

[Mike de Boer [:mikedeboer]]

(In reply to Tim Nguyen :ntim (use needinfo?) from comment #4)
> Looks absolutely great! :D How did you get the sound ?

Uhh, it just worked? :P

Comment 6

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1313325 - support many more theme properties and browser overrides to support more Chrome themes, video files as background for about:home and about:newtab and much more.

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

Comment 7

[Mike de Boer [:mikedeboer]]

With the patch applied to cedar that's currently sitting in ReviewBoard, you can unzip & load up this < https://drive.google.com/file/d/0B0yISxh9PbmsMmlRdXVqZVRieGs/view?usp=sharing > theme to see something interesting:
 - Big Buck Bunny trailer playing in the background of about:home and about:newtab
 - gradient rotating dynamically as time passes
 - generally, horrible color palette.

Alternatively, with this < https://drive.google.com/file/d/0B0yISxh9PbmsbUVGd25TUG1oOHc/view?usp=sharing > theme you can see a random *free* theme from the Chrome store working in Fx.

Comment 8

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

Comment on attachment 8809076 [details]
Bug 1313325 - support many more theme properties and browser overrides to support more Chrome themes, video files as background for about:home and about:newtab and much more.

https://reviewboard.mozilla.org/r/91730/#review92180

::: browser/base/content/newtab/newTab.css:309
(Diff revision 1)
>    opacity: 0;
>    transition: opacity 100ms ease-out;
>  }
>  
>  .newtab-control:-moz-focusring,
> -.newtab-cell:not([ignorehover]) > .newtab-site:hover > .newtab-control {
> +.newtab-cell:not([ignorehover]) > .newtab-siteg > .newtab-control {

this looks like a typo

::: browser/components/extensions/ext-theme.js:236
(Diff revision 1)
>  
> +var gContentScriptLoaded = false;
> +var gContentScript = `function() {
> +  var Theme = {
> +    init() {
> +      addEventListener("DOMContentLoaded", this);

We should pause/play the video on pagehiding/pageshowing, respectively.

::: browser/components/extensions/ext-theme.js:264
(Diff revision 1)
> +      this.video.autoplay = true;
> +      this.video.loop = true;

I think we should mute the video too. I don't think we should allow audio on the video, it will be very distracting lol. A subtle, slow video might look pretty good though.

::: browser/components/extensions/ext-theme.js:313
(Diff revision 1)
>        let val = colors[color];
>        // Since values are optional, they may be `null`.
>        if (val === null) {
>          continue;
>        }
> +      let CSSVarMap = this.cssVars;

nit, we usually don't uppercase the first letter unless it's a const. so this should be cssVarMap.

::: browser/components/extensions/ext-theme.js:405
(Diff revision 1)
>        let val = properties[property];
>        // Since values are optional, they may be `null`.
>        if (val === null) {
>          continue;
>        }
> +      let CSSVarMap = this.cssVars;

ditto.

::: browser/components/extensions/ext-theme.js:579
(Diff revision 1)
> +      browserStyles.push(`
> +        body {
> +          background-size: 100% !important;
>          }`);
>      }
> +    browserStyles.push(`

I think we should add a comment to say that this is closing the @-moz-document.

::: browser/components/extensions/ext-theme.js:600
(Diff revision 1)
> -    if (this.userSheetURI) {
> -      styleSheetService.unregisterSheet(this.userSheetURI, styleSheetService.USER_SHEET);
> +    let whichSheet = asUpdate ? "updateSheetURI" : "userSheetURI";
> +    if (this[whichSheet]) {
> +      styleSheetService.unregisterSheet(this[whichSheet], styleSheetService.USER_SHEET);

is updateSheetURI also a user sheet? if so, then the variable name is confusing, because this line and the ones below assume it is a USER_SHEET.

::: browser/components/extensions/test/browser/browser_ext_theme_extreme.js:128
(Diff revision 1)
> +  // Assert.equal(style.backgroundImage, "linear-gradient(136deg, #ffaaff, #aaffff)",
> +  //   "Expected correct background image");

Can you change this to a todo and then file a bug to look deeper at it?

::: toolkit/modules/LightweightThemeConsumer.jsm:119
(Diff revision 1)
> -    root.style.removeProperty("background-color");
> +    root.style.removeProperty("background");
>      if (active) {
>        root.style.color = aData.textcolor || "black";
> -      root.style.backgroundColor = aData.accentcolor || "white";
> +      root.style.background = aData.accentcolor || "white";

Where do we set background now? It seems we are using the long-hand versions instead of shorthand now.

Comment 9

[Mike de Boer [:mikedeboer]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)
> Where do we set background now? It seems we are using the long-hand versions
> instead of shorthand now.

We set this using the LWT update message. I changed this to the longhand property, because I wanted this demo to be able to set a background gradient ('background-image' long-hand) and not just a color value ('background-color' long-hand). To be able to support both, I changed it to the short-hand version.

Comment 10

[Mike de Boer [:mikedeboer]]

Comment on attachment 8809076 [details]
Bug 1313325 - support many more theme properties and browser overrides to support more Chrome themes, video files as background for about:home and about:newtab and much more.

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

Comment 11

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

Comment on attachment 8809076 [details]
Bug 1313325 - support many more theme properties and browser overrides to support more Chrome themes, video files as background for about:home and about:newtab and much more.

https://reviewboard.mozilla.org/r/91730/#review92772

Comment 12

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/projects/cedar/rev/798319d63092cdcd8064f40fc071ca55e8babc58
Bug 1313325 - support many more theme properties and browser overrides to support more Chrome themes, video files as background for about:home and about:newtab and much more. r=jaws

Comment 13

[CosminB]

I’ve installed the themes from Comment 7 but none of them seems to work in the about:home and about:newtab pages. Are these themes outdated?
I used Firefox 53.0a1 (20161121030224) under Windows 7 64-bit and Mac OS X 10.11.

Here is a screenshot with the error from the browser console: http://screencast.com/t/73pxsLNY 
and a video: http://screencast.com/t/JGXmYYo9zrz 

Any thoughts about this Mike?

Comment 14

[Mike de Boer [:mikedeboer]]

Hi Cosmin, I appreciate the interest! This is an experimental feature that you can only try when you clone & build the cedar project branch. We are creating demos to showcase what's possible to anyone interested.
Please stay tuned for more details soon.