960517 - Adjust Australis's browser theme for Windows 8 and up

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Mike de Boer [:mikedeboer]]

+++ This bug was initially created as a clone of Bug #859751 +++

While Australis looks terrific on Windows 7 and Vista, it looks totally out of place on Windows 8. It just doesn't fit in almost every way:
1. Color scheme - gray and white instead of blue.
2. Buttons and text fields - flat instead of bulky.
3. Hover effects - they're mixed because some are hard coded and some are not.

IMPORTANT: this bug is about the browser theme, NOT the toolkit theme work.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1: Windows 8 browser theme adjustments

Comment 2

[Mike de Boer [:mikedeboer]]

Dão, could you maybe throw in a first-pass review to maybe catch a few of the obvious mistakes/ omissions/ oversights I might've put in the patch?
Since I've been staring at the code for a while, I think there might be a couple of those...

Comment 3

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #2)
> Dão, could you maybe throw in a first-pass review to maybe catch a few of
> the obvious mistakes/ omissions/ oversights I might've put in the patch?
> Since I've been staring at the code for a while, I think there might be a
> couple of those...

You uploaded the wrong patch I think.

Comment 4

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1.1: Windows 8 browser theme adjustments

Gosh Tim, you're right!

Comment 5

[Tim Nguyen :ntim]

Some suggestions :
- Restyle .panel-multiview-anchor
- Use the second row of icons as hover effect and "opened" effect (the third row of icons, the blue ones, is actually useless with shorlander's current mockup)

Comment 6

[Tim Nguyen :ntim]

Also, make sure that the assets you updated include the panorama icon :
check bug 888601 .

Comment 7

[Mike de Boer [:mikedeboer]]

I will chop the patch up into byte-size chunks coming Monday. Stay tuned ;)

Comment 8

[Mike de Boer [:mikedeboer]]

Attachment: Part 1: Windows 8 style toolbar icons

Comment 9

[Dão Gottwald [:dao]]

(In reply to Mike de Boer [:mikedeboer] from comment #7)
> I will chop the patch up into byte-size chunks coming Monday. Stay tuned ;)

That's great, thanks.

Comment 10

[Mike de Boer [:mikedeboer]]

Attachment: Part 2: Windows 8 style menu-panel buttons

Comment 11

[Mike de Boer [:mikedeboer]]

Attachment: Part 2.1: Windows 8 style menu-panel buttons

forgot to hg add a file.

Comment 12

[Mike de Boer [:mikedeboer]]

Attachment: Part 1.1: Windows 8 style toolbar icons

Unbitrotted.

Comment 13

[Mike de Boer [:mikedeboer]]

Attachment: Part 3: Windows 8 style downloads panel

Comment 14

[Mike de Boer [:mikedeboer]]

Attachment: Part 4: Windows 8 style Places Organizer

Comment 15

[Mike de Boer [:mikedeboer]]

Attachment: Part 5: Windows 8 style tabs

Comment 16

[Mike de Boer [:mikedeboer]]

Attachment: Part 6: Windows 8 style main window

Comment 17

[Mike de Boer [:mikedeboer]]

Attachment: Part 7: Windows 8 style toolbars

Comment 18

[Mike de Boer [:mikedeboer]]

Attachment: Part 2.2: Windows 8 style menu-panel buttons

Comment 19

[Dão Gottwald [:dao]]

Comment on attachment 8367856 [details] [diff] [review]
Part 6: Windows 8 style main window

>@@ -189,17 +188,28 @@
>   }
> 
>   #appcontent:not(:-moz-lwtheme) {
>     background-color: -moz-dialog;
>   }
> 
>   #main-menubar:not(:-moz-lwtheme):not(:-moz-window-inactive) {
>     background-color: rgba(255,255,255,.5);
>-    border-radius: 4px;
>+  }
>+
>+  @media (-moz-os-version: windows-vista),
>+         (-moz-os-version: windows-win7) {
...

If you added showfunc = 1 to your diff options in ~/.hgrc, I could easily tell what media block you're modifying here.

> @media not all and (-moz-windows-compositor) {
>   @media (-moz-windows-default-theme) {
>-    #main-window {
>-      background-color: rgb(185,209,234);
>-    }
>-    #main-window:-moz-window-inactive {
>-      background-color: rgb(215,228,242);
>+    @media (-moz-os-version: windows-vista),
>+           (-moz-os-version: windows-win7) {
>+      #main-window {
>+        background-color: rgb(185,209,234);
>+      }
>+      #main-window:-moz-window-inactive {
>+        background-color: rgb(215,228,242);
>+      }
>     }

Is it possible to disable the compositor on Windows 8 by enabling something like Aero Basic on Windows 7? If possible on Windows 8, I have no idea what difference this makes visually and whether not setting the above background-colors is the right thing to do.

Comment 20

[Dão Gottwald [:dao]]

Comment on attachment 8367853 [details] [diff] [review]
Part 5: Windows 8 style tabs

>+%else
>+/* Background tab separators (3px wide) for XP */
>+#tabbrowser-tabs[movingtab] > .tabbrowser-tab[beforeselected]:not([last-visible-tab])::after,
>+.tabbrowser-tab:not([selected]):not([afterselected-visible]):not([afterhovered]):not([first-visible-tab]):not(:hover)::before,
>+#tabbrowser-tabs:not([overflow]) > .tabbrowser-tab[last-visible-tab]:not([selected]):not([beforehovered]):not(:hover)::after {
>+  background-image: url(chrome://browser/skin/tabbrowser/tab-separator-XP.png);
>+}
>+
> /* Use lighter colors of buttons and text in the titlebar on luna-blue */
> @media (-moz-windows-theme: luna-blue) {
>   #main-window[tabsintitlebar]:not([inFullscreen]) .tabbrowser-arrowscrollbox > .scrollbutton-up,
>   #main-window[tabsintitlebar]:not([inFullscreen]) .tabbrowser-arrowscrollbox > .scrollbutton-down {
>     list-style-image: url(chrome://browser/skin/tabbrowser/tab-arrow-left-inverted.png);
>   }
> 
>   #main-window[tabsintitlebar]:not([inFullscreen]) .tabs-newtab-button,
>diff --git a/browser/themes/windows/jar.mn b/browser/themes/windows/jar.mn
>--- a/browser/themes/windows/jar.mn
>+++ b/browser/themes/windows/jar.mn
>@@ -165,17 +165,17 @@ browser.jar:
> #       Makefile.in with a non-default marker of "%" and the result of that gets packaged.
>         skin/classic/browser/tabbrowser/tab-selected-end.svg         (tab-selected-end.svg)
>         skin/classic/browser/tabbrowser/tab-selected-start.svg       (tab-selected-start.svg)
> 
>         skin/classic/browser/tabbrowser/tab-stroke-end.png           (tabbrowser/tab-stroke-end.png)
>         skin/classic/browser/tabbrowser/tab-stroke-start.png         (tabbrowser/tab-stroke-start.png)
>         skin/classic/browser/tabbrowser/tabDragIndicator.png         (tabbrowser/tabDragIndicator.png)
>         skin/classic/browser/tabbrowser/tab-separator-luna-blue.png  (tabbrowser/tab-separator-luna-blue.png)
>-        skin/classic/browser/tabbrowser/tab-separator.png            (tabbrowser/tab-separator.png)
>+        skin/classic/browser/tabbrowser/tab-separator-XP.png         (tabbrowser/tab-separator-XP.png)

I see no reason why this should be packaged as tab-separator-XP.png rather than tab-separator.png. Going with the latter name will allow browser/themes/shared/tabs.inc.css to automatically use that image, removing the need for the above CSS rule.

Comment 21

[Mike de Boer [:mikedeboer]]

Attachment: Part 1.2: Windows 8 style toolbar icons

Comment 22

[Mike de Boer [:mikedeboer]]

Attachment: Part 5.1: Windows 8 style tabs

Comment 23

[Mike de Boer [:mikedeboer]]

Attachment: Part 6.1: Windows 8 style main window

Comment 24

[Mike de Boer [:mikedeboer]]

Attachment: Part 7.1: Windows 8 style toolbars

Comment 25

[Mike de Boer [:mikedeboer]]

Attachment: Part 2.3: Windows 8 style menu-panel buttons

Comment 26

[Mike de Boer [:mikedeboer]]

Attachment: Part 3.1: Windows 8 style downloads panel

Comment 27

[Mike de Boer [:mikedeboer]]

Attachment: Part 4.1: Windows 8 style Places Organizer

Comment 28

[Dão Gottwald [:dao]]

Comment on attachment 8368481 [details] [diff] [review]
Part 6.1: Windows 8 style main window

see comment 19

Comment 29

[Mike de Boer [:mikedeboer]]

(In reply to Dão Gottwald [:dao] from comment #19)
> Is it possible to disable the compositor on Windows 8 by enabling something
> like Aero Basic on Windows 7? If possible on Windows 8, I have no idea what
> difference this makes visually and whether not setting the above
> background-colors is the right thing to do.

It's apparently not possible to disable the compositor (DWM) on Windows 8 - http://msdn.microsoft.com/en-us/library/hh848042%28v=vs.85%29.aspx.
Sometimes a program can be displayed with the 'old' style as if composition were disabled, but it is in fact not. http://stackoverflow.com/questions/13831997/is-dwmiscompositionenabled-still-of-use-in-windows-8

Comment 30

[Dão Gottwald [:dao]]

Comment on attachment 8368481 [details] [diff] [review]
Part 6.1: Windows 8 style main window

> @media not all and (-moz-windows-compositor) {
>   @media (-moz-windows-default-theme) {
>-    #main-window {
>-      background-color: rgb(185,209,234);
>-    }
>-    #main-window:-moz-window-inactive {
>-      background-color: rgb(215,228,242);
>+    @media (-moz-os-version: windows-vista),
>+           (-moz-os-version: windows-win7) {
>+      #main-window {
>+        background-color: rgb(185,209,234);
>+      }
>+      #main-window:-moz-window-inactive {
>+        background-color: rgb(215,228,242);
>+      }
>     }

So this change appears to be unnecessary.

Comment 31

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #25)
> Created attachment 8368483 [details] [diff] [review]
> Part 2.3: Windows 8 style menu-panel buttons

I'd like to see panel subview anchor styled after bug 882807 is landed.

(In reply to Mike de Boer [:mikedeboer] from comment #21)
> Created attachment 8368479 [details] [diff] [review]
> Part 1.2: Windows 8 style toolbar icons

This should use latest icons from the new email link icon bug (in case it hasn't).

Comment 32

[Mike de Boer [:mikedeboer]]

Attachment: Part 6.2: Windows 8 style main window

Comment 33

[Dão Gottwald [:dao]]

Comment on attachment 8368484 [details] [diff] [review]
Part 3.1: Windows 8 style downloads panel

>+%ifdef WINDOWS_AERO
>+/* Win8 and beyond. */
>+@media not all and (-moz-os-version: windows-vista) {
>+  @media not all and (-moz-os-version: windows-win7) {
>+    #downloadsHistory {
>+      transition-duration: 150ms;
>+      transition-property: background-color;
>+      color: ButtonText;
>+    }
>+
>+    #downloadsHistory:hover {
>+      background-color: hsla(210,4%,10%,.05);
>+    }
>+
>+    #downloadsHistory:hover:active {
>+      background-color: hsla(210,4%,10%,.1);
>+      box-shadow: 0 2px 0 0 hsla(210,4%,10%,.1) inset;
>+    }
>+  }
>+}
>+%endif

Should this really be Win8-specific? If so, what about classic themes on Windows 8?

> @media (-moz-windows-default-theme) {
>   #downloadsPanel[hasdownloads] > #downloadsFooter {
>+    background-color: hsla(210,4%,10%,.04);
>+    box-shadow: 0 1px 0 hsla(210,4%,10%,.08) inset;
>+  }
> %ifdef WINDOWS_AERO
>-    background-color: #f1f5fb;
>+  @media (-moz-os-version: windows-vista),
>+         (-moz-os-version: windows-win7) {
>+    #downloadsPanel[hasdownloads] > #downloadsFooter {
>+      background-color: #f1f5fb;
> %else
>+  #downloadsPanel[hasdownloads] > #downloadsFooter {
>     background-color: hsla(216,45%,88%,.98);
> %endif
>     box-shadow: 0px 1px 2px rgb(204,214,234) inset;
>     border-bottom-left-radius: 3px;
>     border-bottom-right-radius: 3px;
>   }
>+%ifdef WINDOWS_AERO
>+}
>+%endif
> }

And this? (Note that this does exclude classic.)

Actually marking r- because I find the above block extremely hard to read... Can you somehow unwind this, even if this means duplicating lines?

Comment 34

[Dão Gottwald [:dao]]

Comment on attachment 8368479 [details] [diff] [review]
Part 1.2: Windows 8 style toolbar icons

>-        skin/classic/aero/browser/Toolbar.png                        (Toolbar-aero.png)
>+        skin/classic/aero/browser/Toolbar.png
>         skin/classic/aero/browser/Toolbar-inverted.png
>+        skin/classic/aero/browser/Toolbar-aero.png

This doesn't look right. Toolbar-aero.png should be packaged as Toolbar.png, and then browser/themes/shared/toolbarbuttons.inc.css will take care of it automatically (so you can drop the rules referring to Toolbar-aero.png).

The new Win8-specific Toolbar.png contains unused states. I think we should drop them from the image until we use them. It seems debatable whether we'll ever want to use them... Seem like we could use opacity for the non-hover/hover distinction and achieve a similar effect with a much lower maintenance burden.

Do we really need the spit between (the new, Win8-specific) Toolbar-inverted.png and Toolbar-inverted-XPVista7.png? Note that Toolbar-inverted.png has much weaker outlines around the icons which I don't think will work well enough on noisy lightweight themes. It contains unused states as well.

Comment 35

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [:dao] from comment #34)
> Comment on attachment 8368479 [details] [diff] [review]
> Part 1.2: Windows 8 style toolbar icons
> 
> >-        skin/classic/aero/browser/Toolbar.png                        (Toolbar-aero.png)
> >+        skin/classic/aero/browser/Toolbar.png
> >         skin/classic/aero/browser/Toolbar-inverted.png
> >+        skin/classic/aero/browser/Toolbar-aero.png
> 
> This doesn't look right. Toolbar-aero.png should be packaged as Toolbar.png,
> and then browser/themes/shared/toolbarbuttons.inc.css will take care of it
> automatically (so you can drop the rules referring to Toolbar-aero.png).

Ah, but then you'd need a different file name and CSS rule for the Win8-specific image.

Please use these kind of overrides to handle this:
http://hg.mozilla.org/mozilla-central/annotate/262e73a6b7cd/browser/themes/osx/jar.mn#l419
This will basically cause bug 702558 on Windows. It's ugly, but third-party themes can work around it. At this point I think this is the lesser evil compared to the CSS getting out of hand here.

Comment 36

[Dão Gottwald [:dao]]

Comment on attachment 8368480 [details] [diff] [review]
Part 5.1: Windows 8 style tabs

>+@media (-moz-os-version: windows-vista),
>+       (-moz-os-version: windows-win7) {
>+  /* Background tab separators (3px wide) for Vista and 7 */
>+  #tabbrowser-tabs[movingtab] > .tabbrowser-tab[beforeselected]:not([last-visible-tab])::after,
>+  .tabbrowser-tab:not([selected]):not([afterselected-visible]):not([afterhovered]):not([first-visible-tab]):not(:hover)::before,
>+  #tabbrowser-tabs:not([overflow]) > .tabbrowser-tab[last-visible-tab]:not([selected]):not([beforehovered]):not(:hover)::after {
>+    background-image: url(chrome://browser/skin/tabbrowser/tab-separator-aero.png);
>+  }
>+}

See comment 35.

Comment 37

[Mike de Boer [:mikedeboer]]

Attachment: Part 1.3: Windows 8 style toolbar icons

Comment 38

[Mike de Boer [:mikedeboer]]

Dão, the `override` statements do not work with a condition like `os=WINNT osversion<8`, `os=WINNT osversion>5 osversion<8`, `os=WINNT osversion<=7` (these I tested).

Is this supposed to work at all on Windows? Do I need to flip a switch somewhere to get this working?

Comment 39

[:Gijs (out for now; he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #38)
> Dão, the `override` statements do not work with a condition like `os=WINNT
> osversion<8`, `os=WINNT osversion>5 osversion<8`, `os=WINNT osversion<=7`
> (these I tested).
> 
> Is this supposed to work at all on Windows? Do I need to flip a switch
> somewhere to get this working?

You should use the win nt version, so 5.1 for XP, 6.2 for 8, etc.

Comment 40

[Mike de Boer [:mikedeboer]]

Aha! Thanks Gijs!

/me slaps forehead.

Comment 41

[Mike de Boer [:mikedeboer]]

Attachment: Part 1.4: Windows 8 style toolbar icons

Comment 42

[Mike de Boer [:mikedeboer]]

Attachment: Part 5.2: Windows 8 style tabs

Comment 43

[Mike de Boer [:mikedeboer]]

Attachment: Part 2.4: Windows 8 style menu-panel buttons

Comment 44

[Mike de Boer [:mikedeboer]]

Attachment: Part 3.2: Windows 8 style downloads panel

Comment 45

[Peter Henkel [:Terepin]]

Will this land in Aurora as well?

Comment 46

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

Yes.

Comment 47

[Mike de Boer [:mikedeboer]]

Attachment: Part 3.3: Windows 8 style downloads panel

Comment 48

[Mike de Boer [:mikedeboer]]

Attachment: Part 7.2: Windows 8 style toolbars

Comment 49

[Benjamin Kerensa [:bkerensa]]

Will track this for now since we want to get Australis right on Windows 8.

Comment 50

[Dão Gottwald [:dao]]

Comment on attachment 8374062 [details] [diff] [review]
Part 1.4: Windows 8 style toolbar icons

The new Toolbar-inverted.png still seems too faint to work well on noisy lightweight theme backgrounds.

Comment 51

[Dão Gottwald [:dao]]

Comment on attachment 8374063 [details] [diff] [review]
Part 5.2: Windows 8 style tabs

>-        skin/classic/browser/tabbrowser/tab-separator.png            (tabbrowser/tab-separator.png)
>+        skin/classic/browser/tabbrowser/tab-separator-XPVista7.png   (tabbrowser/tab-separator-XP.png)

>-        skin/classic/aero/browser/tabbrowser/tab-separator.png       (tabbrowser/tab-separator-aero.png)
>+        skin/classic/aero/browser/tabbrowser/tab-separator.png       (tabbrowser/tab-separator.png)
>+        skin/classic/aero/browser/tabbrowser/tab-separator-XPVista7.png  (tabbrowser/tab-separator-aero.png)

>+% override chrome://browser/skin/tabbrowser/tab-separator.png    chrome://browser/skin/tabbrowser/tab-separator-XPVista7.png    os=WINNT osversion<6.2

This is pretty confusing. Can you please:

- package tab-separator-XP.png as tab-separator.png
- package tab-separator-aero.png as tab-separator-aero.png
- make the override not apply to XP

Comment 52

[Dão Gottwald [:dao]]

Comment on attachment 8374072 [details] [diff] [review]
Part 2.4: Windows 8 style menu-panel buttons

>+    #edit-controls@inAnyPanel@,
>+    #zoom-controls@inAnyPanel@ {
>+      border-radius: 0;
>+      height: 36px;

Why do you need to specify a height here?

>+    #edit-controls@inAnyPanel@ > toolbarbutton,
>+    #zoom-controls@inAnyPanel@ > toolbarbutton {
>+      min-width: calc(@menuPanelWidth@ / 3);
>+      max-width: calc(@menuPanelWidth@ / 3);
>+    }
>+
>+    #edit-controls@inAnyPanel@ > #copy-button,
>+    #zoom-controls@inAnyPanel@ > #zoom-reset-button {
>+      /* reduce the width with 2px for this button to compensate for two separators
>+         of 1px. */
>+      min-width: calc(@menuPanelWidth@ / 3 - 2px);
>+      max-width: calc(@menuPanelWidth@ / 3 - 2px);
>+    }

This seems out of sync with panelUIOverlay.inc.css, which uses @menuPanelButtonWidth@. I'm also concerned about this code rotting again when somebody touches panelUIOverlay.inc.css. Can the separator styling be refactored to avoid the need for this?

Comment 53

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #50)
> Comment on attachment 8374062 [details] [diff] [review]
> Part 1.4: Windows 8 style toolbar icons
> 
> The new Toolbar-inverted.png still seems too faint to work well on noisy
> lightweight theme backgrounds.

He could target the second row of icons instead of the first row either by cropping the image, either by using -moz-image-region.

Comment 54

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen [:ntim] from comment #53)
> (In reply to Dão Gottwald [:dao] from comment #50)
> > Comment on attachment 8374062 [details] [diff] [review]
> > Part 1.4: Windows 8 style toolbar icons
> > 
> > The new Toolbar-inverted.png still seems too faint to work well on noisy
> > lightweight theme backgrounds.
> 
> He could target the second row of icons instead of the first row either by
> cropping the image, either by using -moz-image-region.

That's still not good enough, as the icons have a much weaker outline than in the current Toolbar-inverted.png. I don't think we need this to be Win8-specific at all.

Comment 55

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #54)
> (In reply to Tim Nguyen [:ntim] from comment #53)
> > (In reply to Dão Gottwald [:dao] from comment #50)
> > > Comment on attachment 8374062 [details] [diff] [review]
> > > Part 1.4: Windows 8 style toolbar icons
> > > 
> > > The new Toolbar-inverted.png still seems too faint to work well on noisy
> > > lightweight theme backgrounds.
> > 
> > He could target the second row of icons instead of the first row either by
> > cropping the image, either by using -moz-image-region.
> 
> That's still not good enough, as the icons have a much weaker outline than
> in the current Toolbar-inverted.png. I don't think we need this to be
> Win8-specific at all.

I think it looks fine with the second row of icons in a lightweight theme.
I don't really see the problem. You might want to ask Stephen about this. I'll attach a screen right now.

Comment 56

[Tim Nguyen :ntim]

Attachment: Screenshot of Windows 8 icons in dark lightweight theme

Note that only the navbar has the windows 8 icons in the screenshot. The download icon doesn't use the windows 8 icon either.

Comment 57

[Mike de Boer [:mikedeboer]]

Attachment: Windows 8 browser theme with Light lwtheme applied

Comment 58

[Mike de Boer [:mikedeboer]]

Attachment: Windows 8 browser theme with Dark lwtheme applied

Comment 59

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Looking at the Windows 8 version and comparing it with OS X with the same themes, I don't see that much of a difference in legibility. Some themes are just horrible in that respect, but that doesn't appear to be platform specific.

Dão, do you have a particular theme in mind where the Win 8 version is much worse than others?

Comment 60

[Dão Gottwald [:dao]]

(In reply to Philipp Sackl [:phlsa] from comment #59)
> Looking at the Windows 8 version and comparing it with OS X with the same
> themes, I don't see that much of a difference in legibility. Some themes are
> just horrible in that respect, but that doesn't appear to be platform
> specific.
> 
> Dão, do you have a particular theme in mind where the Win 8 version is much
> worse than others?

No particular theme, just generally themes with a dark base color (so they get inverted icons) but elements that aren't dark. Attachment 8377161 [details] is actually an example for this: The star icon seems to fade away in from of the flowers. I'm pretty sure the current Toolbar-inverted.png does a better job in such cases.

Comment 61

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [:dao] from comment #52)
> Comment on attachment 8374072 [details] [diff] [review]
> Part 2.4: Windows 8 style menu-panel buttons
> 
> >+    #edit-controls@inAnyPanel@ > #copy-button,
> >+    #zoom-controls@inAnyPanel@ > #zoom-reset-button {
> >+      /* reduce the width with 2px for this button to compensate for two separators
> >+         of 1px. */
> >+      min-width: calc(@menuPanelWidth@ / 3 - 2px);
> >+      max-width: calc(@menuPanelWidth@ / 3 - 2px);
> >+    }
> 
> This seems out of sync with panelUIOverlay.inc.css, which uses
> @menuPanelButtonWidth@. I'm also concerned about this code rotting again
> when somebody touches panelUIOverlay.inc.css. Can the separator styling be
> refactored to avoid the need for this?

Alternatively, since the separators for Customize / Help / Exit already use a similar style, should this change just be made across platforms in panelUIOverlay.inc.css?

Comment 62

[Justin Dolske [:Dolske]]

If we're down to tweaking icon/color contrast levels, can we just address that in a followup?

This is a pretty big 7-part patch, a P1 for one of our primary platforms. I'm quite keen to get this landed ASAP, so that we have time to find&fix any other issues that might be found from wider testing.

Comment 63

[Dão Gottwald [:dao]]

(In reply to Justin Dolske [:Dolske] from comment #62)
> If we're down to tweaking icon/color contrast levels, can we just address
> that in a followup?

Landing what's good to go and moving the problematic parts to followup bugs such that we don't land regressions makes more sense to me.

Comment 64

[Mike de Boer [:mikedeboer]]

(In reply to Dão Gottwald [:dao] from comment #51)
> This is pretty confusing. Can you please:
> 
> - package tab-separator-XP.png as tab-separator.png
> - package tab-separator-aero.png as tab-separator-aero.png
> - make the override not apply to XP

Dão, I tried overrides with the following conditions:
```
os=WINNT osversion>=6 osversion<6.2
os=WINNT osversion>5.3 osversion<6.2
```

and both do NOT work. Any suggestions as to how I can make this work?

Comment 65

[Mike de Boer [:mikedeboer]]

(In reply to Dão Gottwald [:dao] from comment #61)
> Alternatively, since the separators for Customize / Help / Exit already use
> a similar style, should this change just be made across platforms in
> panelUIOverlay.inc.css?

Yes! But I'm not particularly fond of doing that in this bug, but instead in a follow-up bug immediately after this one. Don't you agree?

Comment 66

[Mike de Boer [:mikedeboer]]

Comment on attachment 8374062 [details] [diff] [review]
Part 1.4: Windows 8 style toolbar icons

As per comment 62 and comment 64

Comment 67

[Dão Gottwald [:dao]]

(In reply to Mike de Boer [:mikedeboer] from comment #64)
> (In reply to Dão Gottwald [:dao] from comment #51)
> > This is pretty confusing. Can you please:
> > 
> > - package tab-separator-XP.png as tab-separator.png
> > - package tab-separator-aero.png as tab-separator-aero.png
> > - make the override not apply to XP
> 
> Dão, I tried overrides with the following conditions:
> ```
> os=WINNT osversion>=6 osversion<6.2
> os=WINNT osversion>5.3 osversion<6.2
> ```
> 
> and both do NOT work. Any suggestions as to how I can make this work?

If combinding osversion conditions doesn't work (which we should probably file a bug on), can use two overrides, one with osversion=6.0 and one with osversion=6.1?

(In reply to Mike de Boer [:mikedeboer] from comment #65)
> (In reply to Dão Gottwald [:dao] from comment #61)
> > Alternatively, since the separators for Customize / Help / Exit already use
> > a similar style, should this change just be made across platforms in
> > panelUIOverlay.inc.css?
> 
> Yes! But I'm not particularly fond of doing that in this bug, but instead in
> a follow-up bug immediately after this one. Don't you agree?

No, as letting Win8-specific CSS override the legacy base styling and alter the layout is more complicated than just changing the base styling.

Comment 68

[Mike de Boer [:mikedeboer]]

Attachment: Part 1.5: Windows 8 style toolbar icons

I did not obsolete the previous patch, because I find the clarity of the unfolded overrides highly debatable. I leave it up to you to pick one of the two.

Comment 69

[Dão Gottwald [:dao]]

Comment on attachment 8378943 [details] [diff] [review]
Part 1.5: Windows 8 style toolbar icons

This still seems to mess with Toolbar-inverted.png. Please drop this change and file a followup bug on introducing Win8-specific inverted icons for dark lightweight themes (although I'll say it's not clear to me what should be Win8-specific about them, given that we happily use the same inverted icons for dark lightweight themes across Windows XP, Vista and 7).

Comment 70

[Mike de Boer [:mikedeboer]]

(In reply to Dão Gottwald [:dao] from comment #69)
> Comment on attachment 8378943 [details] [diff] [review]
> Part 1.5: Windows 8 style toolbar icons
> 
> This still seems to mess with Toolbar-inverted.png. Please drop this change
> and file a followup bug on introducing Win8-specific inverted icons for dark
> lightweight themes (although I'll say it's not clear to me what should be
> Win8-specific about them, given that we happily use the same inverted icons
> for dark lightweight themes across Windows XP, Vista and 7).

Well, I'm just following UX's instructions, not blindly of course, and this seems to truly be what they (Stephen, Michael, Philipp) want, all things considered.
I don't feel the need to overrule their judgement, neither do I feel the need to fight you over this issue. I think this is what Justin meant with leaving possible improvements to the contrast of inverted/ non-inverted icons for a follow-up with lower priority than P1.

Needinfo-ing the people I mentioned here to get some closure here and definitive steps forward.

Comment 71

[Mike de Boer [:mikedeboer]]

Attachment: Part 2.5: Windows 8 style menu-panel buttons

Comment 72

[Mike de Boer [:mikedeboer]]

Attachment: Part 5.3: Windows 8 style tabs

Comment 73

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: Comparison of old and new white icons

I could swear I commented on this bug a few hours ago, but my comment isn't here… weird.

Anyway: I looked at the issue again. Next to each other, the legibility of the old icons is slightly better. So as long as the normal icons stay Win8-specific, using the old icons for dark themes for the time being seems to be the better choice.

Comment 74

[Stephen Horlander [:shorlander]]

For now can we just keep using the same general Windows Toolbar-inverted.png (https://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/Toolbar-inverted.png) and file a follow-up for investigating Windows 8 specific icons.

Thanks!

Comment 75

[Dão Gottwald [:dao]]

Comment on attachment 8378972 [details] [diff] [review]
Part 5.3: Windows 8 style tabs

This looks good code-wise, but it seems that the new tab-separator.png won't work with dark high-contrast settings or dark lightweight themes, as it consists only of black pixels at different opacity levels. All other tab-separator images have a light outline to deal with different backgrounds. I'd suggest making tab-separator.png a copy of tab-separator-aero.png for now and filing a bug on replacing it with a proper Win8-specific image.

Comment 76

[Mike de Boer [:mikedeboer]]

Attachment: Part 5.4: Windows 8 style tabs

Comment 77

[Mike de Boer [:mikedeboer]]

Attachment: Part 1.6: Windows 8 style toolbar icons

Comment 78

[Dão Gottwald [:dao]]

Comment on attachment 8378971 [details] [diff] [review]
Part 2.5: Windows 8 style menu-panel buttons

>--- a/browser/themes/shared/customizableui/panelUIOverlay.inc.css
>+++ b/browser/themes/shared/customizableui/panelUIOverlay.inc.css

>-#edit-controls > separator,
>-#zoom-controls > separator {
>+.toolbaritem-combined-buttons > separator {
>   -moz-appearance: none;
>   width: 3px;
>   -moz-box-align: stretch;
>   background-image: linear-gradient(to bottom, hsla(0,0%,100%,0), hsla(0,0%,100%,.3) 40%, hsla(0,0%,100%,.3) 60%, hsla(0,0%,100%,0)),
>                     linear-gradient(to bottom, hsla(210,54%,20%,0), hsla(210,54%,20%,.15) 40%, hsla(210,54%,20%,.15) 60%, hsla(210,54%,20%,0)),
>                     linear-gradient(to bottom, hsla(0,0%,100%,0), hsla(0,0%,100%,.3) 40%, hsla(0,0%,100%,.3) 60%, hsla(0,0%,100%,0));
>   background-size: 1px, 1px, 1px;
>   background-position: 0 0, 1px 0, 2px 0;
>   background-repeat: no-repeat;
> }

This looks like it belongs in browser.css...

Comment 79

[Dão Gottwald [:dao]]

Comment on attachment 8379028 [details] [diff] [review]
Part 1.6: Windows 8 style toolbar icons

> @media (-moz-windows-theme: luna-silver) {
>   :-moz-any(@primaryToolbarButtons@),
>+  #home-button.bookmark-item,
>   #bookmarks-menu-button.toolbarbutton-1 > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon {
>     list-style-image: url("chrome://browser/skin/Toolbar-lunaSilver.png");
>   }

Does http://hg.mozilla.org/mozilla-central/annotate/cf9666cc3f36/browser/themes/windows/browser.css#l785 not override this?

Comment 80

[Mike de Boer [:mikedeboer]]

Attachment: Part 1.7: Windows 8 style toolbar icons

(In reply to Dão Gottwald [:dao] from comment #79)
> Does
> http://hg.mozilla.org/mozilla-central/annotate/cf9666cc3f36/browser/themes/
> windows/browser.css#l785 not override this?

Yes, it does! I fixed it in this patch.

Comment 81

[Dão Gottwald [:dao]]

Comment on attachment 8379027 [details] [diff] [review]
Part 5.4: Windows 8 style tabs

https://hg.mozilla.org/integration/fx-team/rev/0a5a8c27afd7

Comment 82

[Dão Gottwald [:dao]]

Comment on attachment 8369385 [details] [diff] [review]
Part 6.2: Windows 8 style main window

https://hg.mozilla.org/integration/fx-team/rev/6c09cd5ce4c9

Comment 83

[Dão Gottwald [:dao]]

Comment on attachment 8368485 [details] [diff] [review]
Part 4.1: Windows 8 style Places Organizer

https://hg.mozilla.org/integration/fx-team/rev/eecb30167ca4

Comment 84

[Dão Gottwald [:dao]]

Comment on attachment 8378971 [details] [diff] [review]
Part 2.5: Windows 8 style menu-panel buttons

https://hg.mozilla.org/integration/fx-team/rev/88ee67ba1b10

Comment 85

[Dão Gottwald [:dao]]

Comment on attachment 8379046 [details] [diff] [review]
Part 1.7: Windows 8 style toolbar icons

https://hg.mozilla.org/integration/fx-team/rev/6fc68b9a7a98

Comment 86

[Dão Gottwald [:dao]]

Comment on attachment 8374190 [details] [diff] [review]
Part 7.2: Windows 8 style toolbars

>-@media (-moz-windows-default-theme) {
>+@media (-moz-windows-default-theme) and (-moz-os-version: windows-vista),
>+       (-moz-windows-default-theme) and (-moz-os-version: windows-win8) {
>   #navigator-toolbox > toolbar:not(:-moz-lwtheme),
>   #browser-bottombox:not(:-moz-lwtheme),
>   .browserContainer > findbar {
>     background-color: @customToolbarColor@;
>   }

windows-win8 should be windows-win7, except that this was already part of another patch, so it should just go away here.

>+/* Win8 and beyond. */
>+@media not all and (-moz-os-version: windows-vista) {
>+  @media not all and (-moz-os-version: windows-win7) {
>+    #urlbar:not(:-moz-lwtheme),
>+    .searchbar-textbox:not(:-moz-lwtheme) {
>+      background-color: hsla(0,0%,100%,.9);
>+      border: 1px solid transparent;
>+      border-color: hsla(210,54%,20%,.25) hsla(210,54%,20%,.27) hsla(210,54%,20%,.3);
>+      box-shadow: 0 1px 0 hsla(0,0%,0%,.01) inset,
>+                  0 1px 0 hsla(0,0%,100%,.1);
>+      transition-property: margin-left, border-color, background-color;
>+      transition-duration: 200ms;
>+    }
>+
>+    #urlbar:not(:-moz-lwtheme)[focused],
>+    .searchbar-textbox:not(:-moz-lwtheme)[focused] {
>+      background-color: hsla(0,0%,100%,1);
>+      border-color: #4595e5;
>+    }
>+
>+    /* Introducing an additional hover state for the Bookmark button */
>+    #nav-bar .toolbarbutton-1[buttonover] > .toolbarbutton-menubutton-button:hover > .toolbarbutton-icon {
>+      background-color: hsla(210,4%,10%,.08);
>+      -moz-border-end: 1px solid;
>+      -moz-padding-end: 5px;
>+      border-color: hsla(210,4%,10%,.1);
>+    }
>+
>+    /* Introducing a panel opened state */
>+    #nav-bar .toolbarbutton-1[open]:not([disabled]) > .toolbarbutton-icon,
>+    #nav-bar .toolbarbutton-1[open]:not([disabled]) > .toolbarbutton-text,
>+    #nav-bar .toolbarbutton-1[open]:not([disabled]) > .toolbarbutton-badge-container,
>+    #nav-bar .toolbarbutton-1[open] > .toolbarbutton-menubutton-dropmarker:not([disabled]) > .dropmarker-icon {
>+      background-color: #177ee5;
>+      border-color: hsla(210,80%,20%,.1);
>+      box-shadow: 0 1px 0 0 hsla(210,80%,20%,.1) inset;
>+    }
>+
>+    #nav-bar :-moz-any(@primaryToolbarButtons@)[open]:not([disabled]),
>+    #nav-bar #bookmarks-menu-button[open] > .toolbarbutton-menubutton-dropmarker:not([disabled]) > .dropmarker-icon {
>+      list-style-image: url(chrome://browser/skin/Toolbar-inverted.png);
>+    }

Are you sure this doesn't inadequately overload Toolbar-inverted.png? This is quite a different context from where we're otherwise using those icons.

Should this whole block really apply regardless of the theme, e.g. also for high-contrast and classic settings? Not sure how well those are hidden under Windows 8, but they're still available, right?

Forking the button styling for Windows 8 (whereas e.g. XP and 7 manage to share the same styling) makes me sad from a maintenance point of view. Any chance that we can unify this stuff again?

Comment 87

[Marco Bonardo [:mak]]

Comment on attachment 8374153 [details] [diff] [review]
Part 3.3: Windows 8 style downloads panel

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

Well, it's mostly questions at the moment, I'm trying to figure which decisions were taken for the theme.

I applied the patch, the result looks pointing towards the mockup, but it's very different.
The padding/margin of the richlistitems is totally off, the icons are still the old ones (with the rounded outline), there is no separator between the entry and the button in the right as well as they are a single entity.
What's the expected result here, is the plan to just update the selection style?
I figure there is a lot to change to match the mockup, though I can't evaluate if we reached the expected goal without knowing what that is. From my point of view we should at least fix the selection style (as you almost did here) and the margin/padding. The internal details that are more complex since they require deep changes to the binding can be left apart, as well as the icons that need new graphic assets.

Moving to what is here, the selection style looks visually correct, though the mockup has a transition on mousedown, what I see is an immediate color change... Could you please get in touch with shorlander and clarify what's expected?
When I mouseover entries, the upper border of the second item is blurry while in the mockup all of the borders are very sharp, looks like the selection border fights with the richlistitems separator. it was an issue even with the previous style, though since the selection was light blue it was barely noticeable, while here is jumping to my eyes.

::: browser/themes/windows/downloads/allDownloadsViewOverlay-aero.css
@@ +6,5 @@
>  %include allDownloadsViewOverlay.css
>  %undef WINDOWS_AERO
>  
> +@media (-moz-windows-default-theme) and (-moz-os-version: windows-vista),
> +       (-moz-windows-default-theme) and (-moz-os-version: windows-win7) {

Just by comparing the Library and Explorer, the selection style doesn't seem to fit, it looks like windows XP selection. (fwiw, the existing selection is also different than the win8 style, not sure if something regressed it, we used to have a better style there, iirc.)
And it is totally different from any other view in the Library...

I don't think we should touch the Library view in this patch, there should be patch to make all of it consistent at a maximum, not just changing a part of it, let's keep it coherent.
I'd limit this patch to the panel, for which we have a mockup that seems to fit in the OS style, so please revert the changes to this overlay style.

::: browser/themes/windows/downloads/downloads.css
@@ +37,5 @@
> +
> +%ifdef WINDOWS_AERO
> +/* < Win8 */
> +@media (-moz-os-version: windows-vista),
> +       (-moz-os-version: windows-win7) {

I don't think the comment is adding anything useful to the self-documenting media query... you may remove all of those < win8 comments, imo.

@@ +51,5 @@
> +}
> +
> +#downloadsHistory:hover:active {
> +  box-shadow: none;
> +}

why not just moving these rules to downloads-aero.css then and set them only for win8?
since any other platform should be fine as it was, looks like we should just need overriding rules in the -aero.css file for win8 that will change transition-duration, hover and hover:active styles

@@ +229,5 @@
> +/* < Win8 */
> +@media (-moz-os-version: windows-vista),
> +       (-moz-os-version: windows-win7) {
> +%endif
> +#downloadsPanel:not([keyfocus]) > #downloadsListBox > richlistitem[type="download"][state="1"][exists]:hover {

why are we making other OS versions overwrite win8, rather than the opposite, as I suggested above? Is there some specific decision taken for themes to do so?

Comment 88

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6fc68b9a7a98
https://hg.mozilla.org/mozilla-central/rev/88ee67ba1b10
https://hg.mozilla.org/mozilla-central/rev/eecb30167ca4
https://hg.mozilla.org/mozilla-central/rev/0a5a8c27afd7
https://hg.mozilla.org/mozilla-central/rev/6c09cd5ce4c9

Mike de Boer ([e-mail])??? Please fix your hg commit information :)

Comment 89

[Peter Henkel [:Terepin]]

(In reply to Dão Gottwald [:dao] from comment #86)
> Should this whole block really apply regardless of the theme, e.g. also for
> high-contrast and classic settings? Not sure how well those are hidden under
> Windows 8, but they're still available, right?
High contrast themes are accessible within Customization windows, just like Windows 7 has it. Classic style, however, is gone. If there is option to enable it, it's hidden and common user won't find that easily, if at all.

Comment 90

[Tim Nguyen :ntim]

Should I fill follow-ups for those things or should it be done here ?
- Restyle .panel-multiview-anchor (as shown in the mockup)
- Darken icons on hover to match the mockup
- Windows 8 style tab scrollbox, new tab, list all tabs icon (ux-consistency)
- Lighter toolbar color to match the mockup (#F8F8F8)
- Windows 8 style menu button separator (solid separator according to mockup)
- Remove border-radius for customization mode buttons

Comment 91

[Tim Nguyen :ntim]

Note for darkening icons :
if you don't want to include more graphics or don't want to set specific image regions for darker graphics, you might want to use SVG filters.

Comment 92

[Tim Nguyen :ntim]

I also found another issue, the switch to metro button doesn't have the windows 8 style look.

Comment 93

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

Tim, this is great feedback. With the amount of patches in this bug and the fact that some still haven't landed, I think you should wait until this bug is closed and then file a new bug for these other issues.

Comment 94

[:Gijs (out for now; he/him)]

(In reply to Tim Nguyen [:ntim] from comment #90)
> - Windows 8 style menu button separator (solid separator according to mockup)

We consciously decided against this for all platforms because it looks bad when you enable the bookmarks toolbar.

Comment 95

[Aris]

Any ETA when all this will land on Aurora channel?

Comment 96

[Tim Nguyen :ntim]

(In reply to Aris from comment #95)
> Any ETA when all this will land on Aurora channel?

Probably when all the patches land on Nightly. There are still 2 patches left still in review.

Comment 97

[Mike de Boer [:mikedeboer]]

(In reply to Marco Bonardo [:mak] from comment #87)
> why are we making other OS versions overwrite win8, rather than the
> opposite, as I suggested above? Is there some specific decision taken for
> themes to do so?

This is the way we've been implementing the design since bug 859751. Since what Stephen shows in his interactive mockups is what Fx will look like from Windows 8 onward, it makes sense to implement the design as the new default (bottom-up, if you will), contrary to implementing it as exceptions to the past (top-down). This way it'll be easier to keep updating the windows theme. I don't know to what degree we support user styles, but this method also makes it easier for theme builders to change things.

Comment 98

[Marco Bonardo [:mak]]

(In reply to Mike de Boer [:mikedeboer] from comment #97)
> This is the way we've been implementing the design since bug 859751. Since
> what Stephen shows in his interactive mockups is what Fx will look like from
> Windows 8 onward, it makes sense to implement the design as the new default
> (bottom-up, if you will), contrary to implementing it as exceptions to the
> past (top-down). This way it'll be easier to keep updating the windows
> theme. I don't know to what degree we support user styles, but this method
> also makes it easier for theme builders to change things.

So, how should we use -aero.css files, just to overwrite rules for Vista/7 glass? Are we pointing towards their removal?
Btw, please fix the other applicable comments, then I will try to review it again with that decision in mind.

Comment 99

[Mike de Boer [:mikedeboer]]

(In reply to Marco Bonardo [:mak] from comment #98)
> So, how should we use -aero.css files, just to overwrite rules for Vista/7
> glass?

Well, yes, indeed! Don't you think that it's rather difficult to keep thinking of Win XP as the default and considering newer versions as exceptions? Treating newer Windows versions as exceptions, style-wise, sounds rather negative to me, whereas they are generally perceived as better looking, are they not?

> Are we pointing towards their removal?

I don't know if it's reasonable to turn that ship around, but I'd definitely vote aye.

> Btw, please fix the other applicable comments, then I will try to review it
> again with that decision in mind.

I will asap, thanks for those!

Comment 100

[u428464]

(In reply to Mike de Boer [:mikedeboer] from comment #97)
> This is the way we've been implementing the design since bug 859751. Since
> what Stephen shows in his interactive mockups is what Fx will look like from
> Windows 8 onward

I really think the download panel that is shown in the mockup is the one to be used on every platform (the same panel is shown on the OSX mockup FWIW).

Comment 101

[Mike de Boer [:mikedeboer]]

Comment on attachment 8368485 [details] [diff] [review]
Part 4.1: Windows 8 style Places Organizer

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: Library window not looking like Windows 8 native.
Testing completed (on m-c, etc.): baked on m-c for a couple of days.
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a

Comment 102

[Mike de Boer [:mikedeboer]]

Comment on attachment 8369385 [details] [diff] [review]
Part 6.2: Windows 8 style main window

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: Firefox main window not looking like Windows 8 native.
Testing completed (on m-c, etc.): baked on m-c for a couple of days.
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a

Comment 103

[Mike de Boer [:mikedeboer]]

Comment on attachment 8378971 [details] [diff] [review]
Part 2.5: Windows 8 style menu-panel buttons

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: Menu panel buttons without square edged buttons on Windows 8
Testing completed (on m-c, etc.): baked on m-c for a couple of days.
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a

Comment 104

[Mike de Boer [:mikedeboer]]

Comment on attachment 8379027 [details] [diff] [review]
Part 5.4: Windows 8 style tabs

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: Australis tabs not blending with the main window correctly on Windows 8
Testing completed (on m-c, etc.): baked on m-c for a couple of days.
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a

Comment 105

[Mike de Boer [:mikedeboer]]

Comment on attachment 8379046 [details] [diff] [review]
Part 1.7: Windows 8 style toolbar icons

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: No nice Windows 8 style icons!
Testing completed (on m-c, etc.): baked on m-c for a couple of days.
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a

Comment 106

[Mike de Boer [:mikedeboer]]

Pushed the approved patches to Aurora:

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/d6b7ebc6f4d8
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/03c5b809110b
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/8e41458bd06a
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/bbcfad841808
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/40b8aade9424

Comment 107

[Mike de Boer [:mikedeboer]]

Attachment: bug-960517-win8Browser-toolbars.patch

Dão, I removed the blue opened state, after I discussed it with Stephen.

Now I have a different problem though: the unified back/ forward button clip paths do not apply nicely anymore. I *think* this has to do with the fact that I use borders now instead of ONLY box-shadows, but that didn't lead me to a fix.
The down state does use a box-shadow on top.

Do you have any ideas how I can fix this?

Comment 108

[Mike de Boer [:mikedeboer]]

I'm pretty sure it's the use of borders. I'm trying to switch to box-shadow.

Comment 109

[henryfhchan]

A few oddities:

In customize mode, the bottom border of the content area sticks to the bottom of the window instead of shrinking back.

The shadow behind the tabs disappears in customize mode and re-appears when out of it.  The shadow isn't necessary / shouldn't be there for either.

Comment 110

[henryfhchan]

The min/max-restore/close buttons are also non-native.

Comment 111

[Mike de Boer [:mikedeboer]]

Attachment: Part 7.3: Windows 8 style toolbars

Stephen, can you take a look at the SVG clip path for the forward-button?

Comment 112

[Stephen Horlander [:shorlander]]

(In reply to Mike de Boer [:mikedeboer] from comment #111)
> Created attachment 8386964 [details] [diff] [review]
> Part 7.3: Windows 8 style toolbars
> 
> Stephen, can you take a look at the SVG clip path for the forward-button?

Here is the path I used to fix the gap:

<svg:clipPath id="windows-keyhole-forward-clip-path" clipPathUnits="objectBoundingBox">
  <svg:path d="m 0, 0 c .3, .25 .3, .75, 0, 1 l 1 0, 0 -1  z"/>
</svg:clipPath>

Comment 113

[Mike de Boer [:mikedeboer]]

Attachment: Part 7.4: Windows 8 style toolbars

Comment 114

[Mike de Boer [:mikedeboer]]

Attachment: Part 3.4: Windows 8 style downloads panel

Comment 115

[Marco Bonardo [:mak]]

Comment on attachment 8387048 [details] [diff] [review]
Part 3.4: Windows 8 style downloads panel

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

issues found while testing this:
- when the panel is empty there is only the Show all downloads part. While before this was looking like a link, now there is a brief hover effect, but it's so subtle that I can barely tell something is happening. Likely causethe background is white, not grey. We could apply an opaque background in this case, though the arrow will stay white and that may look bad. May you figure with Stephen what's expected in this case?
- for whatever reason I don't see any hover styling on download items :/

apart these the patch looks ok, but I'm not sure why I can't see the hover off-hand

::: browser/themes/windows/downloads/downloads.css
@@ +21,5 @@
>  #downloadsHistory {
> +  background-color: transparent;
> +  transition-duration: 150ms;
> +  transition-property: background-color;
> +  color: ButtonText;

is this needed? doesn't look like there's something else setting a color, and the background is not a button bgcolor

@@ +237,5 @@
>  }
>  
> +#downloadsPanel:not([keyfocus]) > #downloadsListBox > richlistitem[type="download"][state="1"][exists]:hover:active {
> +  background-color: Highlight;
> +  box-shadow: none;

should you also set outline: 0 here?

Comment 116

[Mike de Boer [:mikedeboer]]

(In reply to Marco Bonardo [:mak] from comment #115)
> Comment on attachment 8387048 [details] [diff] [review]
> Part 3.4: Windows 8 style downloads panel
> 
> Review of attachment 8387048 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> issues found while testing this:
> - when the panel is empty there is only the Show all downloads part. While
> before this was looking like a link, now there is a brief hover effect, but
> it's so subtle that I can barely tell something is happening. Likely
> causethe background is white, not grey. We could apply an opaque background
> in this case, though the arrow will stay white and that may look bad. May
> you figure with Stephen what's expected in this case?

I will!

> - for whatever reason I don't see any hover styling on download items :/

I'm not going to introduce them here. The downloads panel is slated for a UX overhaul anyways, so I'm leaving things be as much as possible here.

Comment 117

[Mike de Boer [:mikedeboer]]

Attachment: Part 3.5: Windows 8 style downloads panel

Comment 118

[Justin Dolske [:Dolske]]

Comment on attachment 8387046 [details] [diff] [review]
Part 7.4: Windows 8 style toolbars

Over to jaws so he and Mike can bang this out today.

Comment 119

[Justin Dolske [:Dolske]]

Comment on attachment 8387693 [details] [diff] [review]
Part 3.5: Windows 8 style downloads panel

...ditto!

Comment 120

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

Comment on attachment 8387046 [details] [diff] [review]
Part 7.4: Windows 8 style toolbars

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

this would have been r+ but the vista/7 part wasn't tested due to the syntax error. we need to test the vista/7 changes before this can land.

::: browser/themes/windows/browser-aero.css
@@ +327,5 @@
> +      border: 1px solid transparent;
> +      border-color: hsla(210,54%,20%,.25) hsla(210,54%,20%,.27) hsla(210,54%,20%,.3);
> +      box-shadow: 0 1px 0 hsla(0,0%,0%,.01) inset,
> +                  0 1px 0 hsla(0,0%,100%,.1);
> +      transition-property: margin-left, border-color, background-color;

need to remove the "margin-left, "

@@ +342,5 @@
> +    #nav-bar .toolbarbutton-1[buttonover] > .toolbarbutton-menubutton-button:hover > .toolbarbutton-icon {
> +      background-color: hsla(210,4%,10%,.08);
> +      -moz-border-end: 1px solid;
> +      -moz-padding-end: 5px;
> +      border-color: hsla(210,4%,10%,.1);

there is something wrong with the border color at the beginning of this transition. it appears near black (or -moz-use-text-color?). 

i looked in to this and it wasn't obvious as to why this was creeping in. we can fix this in a follow-up so we don't hold up the rest of this patch for it.

::: browser/themes/windows/browser.css
@@ +551,5 @@
> +  #nav-bar .toolbarbutton-1 > .toolbarbutton-icon,
> +  #nav-bar .toolbarbutton-1 > .toolbarbutton-text,
> +  #nav-bar .toolbarbutton-1 > .toolbarbutton-badge-container,
> +  #nav-bar .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
> +    background-color: hsla(210,32%,93%,0);

it looks like you need a curly bracket here :)

Comment 121

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

Comment on attachment 8387693 [details] [diff] [review]
Part 3.5: Windows 8 style downloads panel

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

this looks good, nice.

Comment 122

[Jacek Caban]

I landed an encoding fixup (revealed by problems with cross compilation):

https://hg.mozilla.org/integration/mozilla-inbound/rev/b97c09e3de3d

Comment 123

[Mike de Boer [:mikedeboer]]

Comment on attachment 8387693 [details] [diff] [review]
Part 3.5: Windows 8 style downloads panel

Checked-in as: https://hg.mozilla.org/integration/fx-team/rev/a18c5fa023cb

Comment 124

[Mike de Boer [:mikedeboer]]

Attachment: Part 7.5: Windows 8 style toolbars

Comment 125

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b97c09e3de3d

Comment 126

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a18c5fa023cb

Comment 127

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

Comment on attachment 8388667 [details] [diff] [review]
Part 7.5: Windows 8 style toolbars

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

::: browser/base/content/browser.xul
@@ +1165,5 @@
>  #include tab-shape.inc.svg
>  
>  #ifndef XP_UNIX
>      <svg:clipPath id="windows-keyhole-forward-clip-path" clipPathUnits="objectBoundingBox">
> +      <svg:path d="m 0, 0 c .3, .25 .3, .75, 0, 1 l 1 0, 0 -1  z"/>

nit please remove spaces between x,y coordinates.
m 0,0 c .3,.25 .3,.75 0,1 l 1,0 0,-l z

::: browser/themes/windows/browser-aero.css
@@ +342,5 @@
> +    #nav-bar .toolbarbutton-1[buttonover] > .toolbarbutton-menubutton-button:hover > .toolbarbutton-icon {
> +      background-color: hsla(210,4%,10%,.08);
> +      -moz-border-end: 1px solid;
> +      -moz-padding-end: 5px;
> +      border-color: hsla(210,4%,10%,.1);

this still has the issue about a dark border color being shown at the start of the transition. please file a follow-up bug about it, we should fix that but i don't want to hold up the rest of this patch for that one issue.

::: browser/themes/windows/browser.css
@@ +544,4 @@
>    border: 1px solid;
> +  border-color: transparent;
> +  transition-property: background-color, border-color;
> +  transition-duration: 250ms;

why are we changing the transition to 250ms from 150ms? we use 150ms on linux now too, and 150ms on <Win8. please change this back to 150ms.

Comment 128

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

I landed your patch for you on fx-team so that it has a chance of making tomorrow's Nightly build.

https://hg.mozilla.org/integration/fx-team/rev/ece9cdfb15d7

Comment 129

[:Gijs (out for now; he/him)]

https://hg.mozilla.org/mozilla-central/rev/a18c5fa023cb

Comment 130

[:Gijs (out for now; he/him)]

(In reply to :Gijs Kruitbosch from comment #129)
> https://hg.mozilla.org/mozilla-central/rev/a18c5fa023cb

Oh blegh, this cset was here already, but the bug wasn't resolved because the *other* cset hasn't yet made it to m-c. Sigh.

Comment 131

[Mike de Boer [:mikedeboer]]

Comment on attachment 8387693 [details] [diff] [review]
Part 3.5: Windows 8 style downloads panel

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: The downloads panel will look a bit alien to Windows 8 users.
Testing completed (on m-c, etc.): baked on m-c for a day.
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a

Comment 132

[Mike de Boer [:mikedeboer]]

Ty, Sylvestre!

https://hg.mozilla.org/releases/mozilla-aurora/rev/11d26367c466

Comment 133

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

Comment on attachment 8388667 [details] [diff] [review]
Part 7.5: Windows 8 style toolbars

*trying to fix the screwiness that bugzilla was yesterday, this should have r=jaws*

Comment 134

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ece9cdfb15d7

Comment 135

[Mike de Boer [:mikedeboer]]

Comment on attachment 8388667 [details] [diff] [review]
Part 7.5: Windows 8 style toolbars

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: The look of buttons on the nav-bar currently have the Windows Vista/ 7 look. This patch changes that for Windows 8 users.
Testing completed (on m-c, etc.): baked on m-c for a day.
Risk to taking this patch (and alternatives if risky): minor
String or IDL/UUID changes made by this patch: n/a

Comment 136

[Mike de Boer [:mikedeboer]]

Pushed to Aurora as: https://hg.mozilla.org/releases/mozilla-aurora/rev/c43a49abc397

Thanks to all for your tremendous amount of help on this bug!

Comment 137

[Cornel Ionce [:noni] [Hubs QA]]

Marking issue verified as all of it's dependencies are verified fixed except for the issues described in the meta bug 982644.

Tested on Windows 8 64bit and on a MS Pro 2 device running Windows 8.1 64bit using:
- latest Aurora, build ID: 20140416004008
- Fx 29 beta 8, build ID: 20140414143035.

Comment 138

[henryfhchan]

All the traditional apps (non metro non ribbon) buttons in windows 8 use a blue background and border on hover, and have a more saturated state on click. The current patch (1) no color, (2) border turns lighter on click (3) extra inset shadow on click instead. 1 is arguably for consistency with australis but 2 and 3 are not found in australis nor Windows itself. Could these be fine tuned? It's a bit jarring with all due respect. On nightly 21 Apr.

Comment 139

[:Gijs (out for now; he/him)]

(In reply to henryfhchan from comment #138)
> All the traditional apps (non metro non ribbon) buttons in windows 8 use a
> blue background and border on hover, and have a more saturated state on
> click. The current patch (1) no color, (2) border turns lighter on click (3)
> extra inset shadow on click instead. 1 is arguably for consistency with
> australis but 2 and 3 are not found in australis nor Windows itself. Could
> these be fine tuned? It's a bit jarring with all due respect. On nightly 21
> Apr.

Please file a new bug for this, ideally with some illustrative screenshots. Thank you!