849223 - Make the CSS3 color system module customizable via mozSettings to enable gaia themes

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: poc

Right now we are pushing building blocks. The current way to do it is for people to embed the CSS in their app directly. The main issue with that beeing that if we change the color of the headers or any other part that won't be reflected and third party apps will look out of place.

This will also affects the notes app and any apps that don't belong to Gaia.

Using CSS 3 colors modules will let us abstract this and so OEM and carriers will be able to customize the default set of colors used in apps. Third party apps will be able to opt-in to it if needed and even websites. That will offer an easy way to look integrated into the system whatever default color theme it is, so the same app will look integrated in various different color theme of Firefox OS without requiring any changes.

That will also make sure people don't fork an application just to have a default color that is different than the orange one.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

For example you can do: |background: menu -moz-linear-gradient(top, rgba(0,0,0,0.5) 0%, rgba(0,0,0,0.5) 100%);| in the section[role="region"] > header:first-child  of the header.css building block.


Then from user.js you can change this list of keywords: http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#198

Comment 2

[Ismael Gonzalez [:basiclines]]

We were not using any CSS3 gradients as there was performance problems with it.
Do we solved that problems?

Thx!

Comment 3

[Ismael Gonzalez [:basiclines]]

So this is working like "variables" for color types?

Comment 4

[Etienne Segonzac (:etienne)]

(In reply to Ismael from comment #2)
> We were not using any CSS3 gradients as there was performance problems with
> it.
> Do we solved that problems?
> 

Not that I know of.

This bug is still at the exploration stage, but we could probably use gradient images made to be layered on top of a background color.

Comment 5

[Ismael Gonzalez [:basiclines]]

(In reply to Etienne Segonzac (:etienne) from comment #4)
> This bug is still at the exploration stage, but we could probably use
> gradient images made to be layered on top of a background color.

Sure a semi-transparent png file will work also!

(In reply to Vivien Nicolas (:vingtetun) (:21) (away until march 25th) from comment #1)
> For example you can do: |background: menu -moz-linear-gradient(top,
> rgba(0,0,0,0.5) 0%, rgba(0,0,0,0.5) 100%);| in the section[role="region"] >
> header:first-child  of the header.css building block.
> 
> 
> Then from user.js you can change this list of keywords:
> http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#198

CSS2 system colors seems to be deprecated in the new CSS3 color module:
http://www.w3.org/TR/css3-color/#css2-system

Comment 6

[Etienne Segonzac (:etienne)]

Attachment: WIP - gecko patch binding keywords <-> mozSettings

With this patch we're binding the color values for those keywords:

    menu
    menuText
    appworkspace
    window
    windowText
    infoBackground
    buttonFace
    highlight

to theirs gaia.ui._keyword_ equivalent

Comment 7

[Etienne Segonzac (:etienne)]

Attachment: Gaia patch to have default values for the colors

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Etienne Segonzac (:etienne) from comment #7)
> Created attachment 726772 [details] [diff] [review]
> Gaia patch to have default values for the colors

Are you waiting for anything else before asking for a r? ?

Comment 9

[Etienne Segonzac (:etienne)]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #8)
> (In reply to Etienne Segonzac (:etienne) from comment #7)
> > Created attachment 726772 [details] [diff] [review]
> > Gaia patch to have default values for the colors
> 
> Are you waiting for anything else before asking for a r? ?

It probably needs a bit of rebasing and reasonable default colors before the review :)

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Etienne Segonzac (:etienne) from comment #9)
> (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #8)
> > (In reply to Etienne Segonzac (:etienne) from comment #7)
> > > Created attachment 726772 [details] [diff] [review]
> > > Gaia patch to have default values for the colors
> > 
> > Are you waiting for anything else before asking for a r? ?
> 
> It probably needs a bit of rebasing and reasonable default colors before the
> review :)

It does not even needs any color define. Just support the definition of those colors via a pref.

Comment 11

[Etienne Segonzac (:etienne)]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #10)
> (In reply to Etienne Segonzac (:etienne) from comment #9)
> > (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #8)
> > > (In reply to Etienne Segonzac (:etienne) from comment #7)
> > > > Created attachment 726772 [details] [diff] [review]
> > > > Gaia patch to have default values for the colors
> > > 
> > > Are you waiting for anything else before asking for a r? ?
> > 
> > It probably needs a bit of rebasing and reasonable default colors before the
> > review :)
> 
> It does not even needs any color define. Just support the definition of
> those colors via a pref.

Well when the setting binded to a pref is undefined (likely when for phones that will get this through updates) we need to have _something_ :)

Comment 12

[Francisco Jordano [:arcturus] [:francisco]]

This poc, made my day :)_

Really glad to see this moving forward, do you need any help guys?

Comment 13

[Julien Wajsberg [:julienw]]

You're talking about http://www.youtube.com/watch?v=Vp5nnJFtkiY ?

Comment 14

[Francisco Jordano [:arcturus] [:francisco]]

<3

Comment 15

[Etienne Segonzac (:etienne)]

Attachment: Gecko part

Reviving this old bug.

If using the (deprecated?) system colors to theme gaia is ok, I'd like to land this and open follow up bugs to update the building blocks during the next visual refresh.

Comment 16

[Etienne Segonzac (:etienne)]

Attachment: Gaia part

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8360492 [details] [diff] [review]
Gecko part

Review of attachment 8360492 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the code moved to settings.js. The list of colors can probably be enhanced later.

::: b2g/chrome/content/shell.js
@@ +1211,5 @@
> +        Services.prefs.setCharPref(pref, value);
> +      }
> +    });
> +  });
> +})();

Can you move this code to b2g/chrome/content/settings.js ?

Comment 18

[Francisco Jordano [:arcturus] [:francisco]]

Thanks folks :)

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8360492 [details] [diff] [review]
Gecko part

Why are the nsLookAndFeel.cpp changes needed?

They look wrong to me.  I thought the setup was that the cross-platform code (widget/xpwidgets/nsXPLookAndFeel.cpp) has a GetColor function that will use the pref value if the pref is set, and if it isn't, will call NativeGetColor.  (It uses the same cache both for the pref values and for memoized results of NativeGetColor, so it's not immediately obvious looking at the code, though.)  So it seems really strange for NativeGetColor to be calling GetColor.  (How does it not cause infinite recursion?  Perhaps because the code is never reached because the prefs are always set?)

Otherwise this seems fine... though at that point there aren't any Gecko changes left.

Comment 20

[Etienne Segonzac (:etienne)]

Attachment: Gecko part v2

Quick r? because the patch changed a bit.
Now it's tiny!

Comment 21

[Etienne Segonzac (:etienne)]

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #19)
> Comment on attachment 8360492 [details] [diff] [review]
> Gecko part
> 
> Why are the nsLookAndFeel.cpp changes needed?
> 
> They look wrong to me.  I thought the setup was that the cross-platform code
> (widget/xpwidgets/nsXPLookAndFeel.cpp) has a GetColor function that will use
> the pref value if the pref is set, and if it isn't, will call
> NativeGetColor. 

Thanks! It works perfectly, patch updated.

Comment 22

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8360990 [details] [diff] [review]
Gecko part v2

Review of attachment 8360990 [details] [diff] [review]:
-----------------------------------------------------------------

Oh that sounds soo easy now...

Comment 23

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Gecko part: https://hg.mozilla.org/integration/mozilla-inbound/rev/72be9c70269a

Please keep opened until the Gaia part lands.

Comment 24

[Etienne Segonzac (:etienne)]

The gaia part just landed here:
https://github.com/mozilla-b2g/gaia/commit/11feeea66b0d144828753874edb17453ce8f31b7

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/72be9c70269a