838244 - Create a theme directory for shared default theme resources

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement)

Description

[Matthew N. [:MattN]]

There is quite a bit of duplication of CSS styles between our default themes (e.g. winstripe/pinstripe/gnomestripe) which makes implementing new features and re-styling old features somewhat tedious and error prone.

I propose that we share more styles between the default themes and split up the shared css files to make them more manageable (similar to how browser.js was recently split up in bug 758812).  We would continue to use browser/base/content/browser.css for structural markup so this would only affect theme-related styles.  To avoid a performance impact, the modularized shared CSS files can be pre-processed into the various OS-theme stylesheets. This also allows them to use #defines of the including stylesheet.

The current proposal is to create browser/themes/shared/ containing files such as browser-devtools.inc.css and browser-social.inc.css.

Comment 1

[Matthew N. [:MattN]]

Attachment: Part 1 - Create themes/shared and move browserShared.inc there

Simple base patch to create the directory with browserShared.inc renamed to shared/browser.inc.

Comment 2

[Matthew N. [:MattN]]

Attachment: Part 2 - Move common highlighter CSS

4 files changed, 168 insertions(+), 402 deletions(-)

I used diffmerge to do a 3-way merge with the three default themes to check for differences.

Comment 3

[Matthew N. [:MattN]]

Attachment: Part 3 - Move common GCLI CSS

4 files changed, 101 insertions(+), 240 deletions(-)

Comment 4

[Matthew N. [:MattN]]

Attachment: Part 4 - Move common responsive design mode CSS

4 files changed, 56 insertions(+), 136 deletions(-)

Comment 5

[Matthew N. [:MattN]]

Attachment: Part 5 - Move common social API chat CSS

4 files changed, 139 insertions(+), 334 deletions(-)

Comment 6

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

Comment on attachment 713772 [details] [diff] [review]
Part 5 - Move common social API chat CSS

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

This is fantastic! Thank you for removing so much duplication!

::: browser/themes/gnomestripe/browser.css
@@ -2274,1 @@
>    margin: -285px -1px 0 -1px;

I'm not sure if you want to make this change here or not, I'll leave it up to you, but this should have the same |margin: -285px 0 0;| as the other themes.

Comment 7

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/fx-team/rev/724ce2b72420
https://hg.mozilla.org/integration/fx-team/rev/44101236921b
https://hg.mozilla.org/integration/fx-team/rev/fcc189143669
https://hg.mozilla.org/integration/fx-team/rev/f4333fce2aeb
https://hg.mozilla.org/integration/fx-team/rev/4863b7e9a769

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/724ce2b72420
https://hg.mozilla.org/mozilla-central/rev/44101236921b
https://hg.mozilla.org/mozilla-central/rev/fcc189143669
https://hg.mozilla.org/mozilla-central/rev/f4333fce2aeb
https://hg.mozilla.org/mozilla-central/rev/4863b7e9a769

Comment 9

[Rob Campbell [:rc] (:robcee)]

thank you!

Comment 10

[Justin Dolske [:Dolske]]

I filed bug 844412 for doing this over in /toolkit too.