859751 - Theme adjustments for Windows 8

Status: VERIFIED FIXED

(Toolkit :: Themes, defect)

Description

[Peter Henkel [:Terepin]]

Attachment: Firefox navbar vs Windows 8 Explorer

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.

Comment 1

[Stephen Horlander [:shorlander]]

Attachment: Australis - Windows 8 - i01

Some short term things we can do to improve this:
- Use -moz-dialog and not the custom blue
- Flatter Windows 8 specific icons and assets
- Remove rounded corners from fields

Longer term things to figure out:
- Toolbar Hover and Pressed states
- Shape and style of secondary UI e.g. Panels, incontent UI

Comment 2

[Peter Henkel [:Terepin]]

Thanks Stephen! One thing though: The close button is actually red the whole time, just like in Win7.

Comment 3

[Jim Mathies [:jimm]]

Is the fx button going away?

Comment 4

[Matthew N. [:MattN]]

(In reply to Jim Mathies [:jimm] from comment #3)
> Is the fx button going away?

Yes, it's already gone (bug 863753) and is being replaced by the app menu on the right of the nav-bar.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Reporter - are there still problems with Australis on Windows 8?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Assigning to self if this is still an issue.

Comment 7

[u428464]

(In reply to Mike Conley (:mconley) from comment #6)
> Assigning to self if this is still an issue.

If I'm right this bug is not about "issues" but about improvements on Win 8 see comment 1.

Comment 8

[Peter Henkel [:Terepin]]

(In reply to Mike Conley (:mconley) from comment #5)
> Reporter - are there still problems with Australis on Windows 8?

There are no problems in Win8 as far as I know. This bug's sole purpose is to adjust Australi's appearance to fit into Windows 8. See mockup.

Comment 9

[Peter Henkel [:Terepin]]

I have one technical question, if I may: Why is that many hover effects are hardcoded and not using system's styling?

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Removing the items from M7 that do not block us from landing on m-c.

Comment 11

[Tim Nguyen :ntim]

Screenshot of userstyle that nearly matches shorlander design (also works with lightweight themes) : http://fc06.deviantart.net/fs70/f/2013/195/7/2/my_firefox___14_07_13_by_ntim007-d6de5tg.png

I can make a few tweaks (to match perfectly shorlander design and removing the userstyle bits) and try making a patch out of it (if I can understand Mozilla patching system, MDN tut isn't helping me).

Comment 12

[Mike de Boer [:mikedeboer]]

Mike, is it OK if I steal this from you?

Comment 13

[Mike Conley (:mconley) (:⚙️)]

(In reply to Mike de Boer [:mikedeboer] from comment #12)
> Mike, is it OK if I steal this from you?

By all means!

Comment 14

[Mike de Boer [:mikedeboer]]

Attachment: WIP Patch: differentiate styling on Win8

Mike, this is my first stab at the issue. It sports a gray-ish navbar, while still supporting transparency/ gradients, thus LWT.

I removed border-radii from the urlbar and search textbox, not in other places. Don't know if that's necessary.

Is this approach a step in the right direction?

Comment 15

[Dão Gottwald [:dao]]

(In reply to Mike de Boer [:mikedeboer] from comment #14)
> Created attachment 784453 [details] [diff] [review]
> WIP Patch: differentiate styling on Win8
> 
> Mike, this is my first stab at the issue. It sports a gray-ish navbar,

*I think* the default -moz-dialog is already what we want here, so we should "just" restrict any rules using @customToolbarColor@ to Windows Vista and 7.

Comment 16

[Mike de Boer [:mikedeboer]]

(In reply to ntim007 from comment #11)
> I can make a few tweaks (to match perfectly shorlander design and removing
> the userstyle bits) and try making a patch out of it (if I can understand
> Mozilla patching system, MDN tut isn't helping me).

That looks rather nice, but it's moving a lot towards the tile view ('metro-style') style, for which we have a whole separate project going. This bug is mainly about how the browser will look on the Desktop, where we usually match the native UI like Windows 8 Explorer; attachment 735108 [details].

Comment 17

[Peter Henkel [:Terepin]]

Can't we just follow Stephen's mockup?

Comment 18

[Mike de Boer [:mikedeboer]]

(In reply to Peter Henkel [:Terepin] from comment #17)
> Can't we just follow Stephen's mockup?

Sure, that's my intention... I was merely posting a WIP patch to get some feedback from my respected and experienced colleagues :) If I were to make it work like Stephen's mockup, I'd like to know I'm following the right approach first.

Comment 19

[Peter Henkel [:Terepin]]

I'm asking because Stephen himself wrote "Use -moz-dialog and not the custom blue".

Comment 20

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #14)
> Created attachment 784453 [details] [diff] [review]
> WIP Patch: differentiate styling on Win8
> 
> Mike, this is my first stab at the issue. It sports a gray-ish navbar, while
> still supporting transparency/ gradients, thus LWT.
> 
> I removed border-radii from the urlbar and search textbox, not in other
> places. Don't know if that's necessary.
> 
> Is this approach a step in the right direction?

Just some feedback :
It's a good start, but it could get some easy improvements :
- Why not 
> customToolbarColorWin8 -moz-dialog
Instead of
> customToolbarColorWin8 hsl(210,0%,92%)

- Why does #navigatorToolbox has a gray background ? it also affects the tab bar. According to shorlander's mockup, the whole titlebar should keep the color drawn by Windows 8

- You should do also apply the new tab "grain" for the toolbar and tab background. It's also shown on shorlander's mockup (it's barely visible), and was pretty visible on it's old windows 8 mockups (http://shorlander.dropmark.com/51486/532405 )

- You could also easily change the toolbar icons to the windows 8 ones using list-style:url(win8/file.png) but it would require shorlander to also make glyphs for the dropdown arrow, the tabview button, the tabbar overflow arrows, and the new tab button

Comment 21

[Tim Nguyen :ntim]

- You could also remove border-radius from panels too

Comment 22

[Matthew N. [:MattN]]

Comment on attachment 784453 [details] [diff] [review]
WIP Patch: differentiate styling on Win8

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

This is another bug where CSS variables would have helped :( We could have used variables to switch colours without having to duplicate so many selectors.

This is more additions that I expected. I thought we would mostly be having pre-aero styling applying to Win8. Less CSS is better.

::: browser/themes/windows/browser-win8.inc
@@ +106,5 @@
> +  }
> +
> +  #urlbar,
> +  .searchbar-textbox {
> +    border-radius: 0;

I think this should probably be done at the toolkit level for all textbox elements

Comment 23

[Mike de Boer [:mikedeboer]]

(In reply to Matthew N. [:MattN] from comment #22)
> I think this should probably be done at the toolkit level for all textbox
> elements

FYI, the Windows toolkit theme doesn't apply radii on borders for textboxes.

Comment 24

[Mike de Boer [:mikedeboer]]

Attachment: Patch v1: differentiate styling on Windows 8 Desktop

Comment 25

[Mike Conley (:mconley) (:⚙️)]

I'll look at this in a bit - but just a heads-up that I believe we also wanted to update the toolbar buttons for Windows 8 so that they behave the same way as on OSX (ie, no hover chrome, colour change to indicate hover:active - the toolbar styles should probably be moved from themes/osx/browser.css to something that can be shared between OSX and Win8).

Comment 26

[Mike de Boer [:mikedeboer]]

(In reply to Mike Conley (:mconley) from comment #25)
> I'll look at this in a bit - but just a heads-up that I believe we also
> wanted to update the toolbar buttons for Windows 8 so that they behave the
> same way as on OSX (ie, no hover chrome, colour change to indicate
> hover:active - the toolbar styles should probably be moved from
> themes/osx/browser.css to something that can be shared between OSX and Win8).

Alright, I'll postpone the review when I added that to the patch. Thanks for the heads-up ;)

Comment 27

[Dão Gottwald [:dao]]

(In reply to Mike Conley (:mconley) from comment #25)
> I'll look at this in a bit - but just a heads-up that I believe we also
> wanted to update the toolbar buttons for Windows 8 so that they behave the
> same way as on OSX

Is that native Windows 8 behavior?

Comment 28

[Peter Henkel [:Terepin]]

Attachment: hover.png

Comment 29

[Peter Henkel [:Terepin]]

Attachment: pressed.png

Comment 30

[Mike Conley (:mconley) (:⚙️)]

I in(In reply to Dão Gottwald [:dao] from comment #27)
> (In reply to Mike Conley (:mconley) from comment #25)
> > I'll look at this in a bit - but just a heads-up that I believe we also
> > wanted to update the toolbar buttons for Windows 8 so that they behave the
> > same way as on OSX
> 
> Is that native Windows 8 behavior?

I inferred this behaviour based on the icons that shorlander gave us, but I'll needinfo him to be sure.

Comment 31

[Matthew N. [:MattN]]

Comment on attachment 786292 [details] [diff] [review]
Patch v1: differentiate styling on Windows 8 Desktop

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

Did you check if when you switch to an older theme on Windows 8 that Windows uses sharp corners? If not, it seems like this patch needs more -moz-windows-default-theme. I'm not at a Windows 8 computer at the moment to test.

::: browser/themes/windows/browser-aero.css
@@ +108,5 @@
>    #main-window[sizemode="maximized"] #titlebar-buttonbox {
>      -moz-margin-end: 3px;
>    }
>  
> +

Nit: Intentional?

@@ +307,5 @@
> +  }
> +
> +  #urlbar,
> +  .searchbar-textbox {
> +    border-radius: 0;

Did putting this in toolkit or layout not work out?

Comment 32

[Dão Gottwald [:dao]]

(In reply to Matthew N. [:MattN] from comment #31)
> > +  #urlbar,
> > +  .searchbar-textbox {
> > +    border-radius: 0;
> 
> Did putting this in toolkit or layout not work out?

The existing radius is from browser.css. It's no toolkit business.

Comment 33

[Tim Nguyen :ntim]

(In reply to Mike Conley (:mconley) from comment #30)
> I in(In reply to Dão Gottwald [:dao] from comment #27)
> > (In reply to Mike Conley (:mconley) from comment #25)
> > > I'll look at this in a bit - but just a heads-up that I believe we also
> > > wanted to update the toolbar buttons for Windows 8 so that they behave the
> > > same way as on OSX
> > 
> > Is that native Windows 8 behavior?
> 
> I inferred this behaviour based on the icons that shorlander gave us, but
> I'll needinfo him to be sure.

I personally think that the top sprite (the gray icons) is for the navbar.
The second sprite (the black ones) is for the tabbar (as you see in shorlander mockup, they look a bit darker in the tabbar). The third sprite (the blue icons), are for [open] states.

I think the behaviour isn't like mac, because as you see http://shorlander.dropmark.com/118810/1831621 , there's a background behind the toolbar button-icon.

Comment 34

[Tim Nguyen :ntim]

Attachment: Screenshot of userstyle matching 80% of shorlander's mockup

I updated the userstyle a little bit to match shorlander's mockup better. I'll attach the code later, so you guys can play around with it and maybe use some parts of it :)

Comment 35

[Tim Nguyen :ntim]

Attachment: Userstyle matching 80% of shorlander's mockup

Here's the code of the userstyle. I know it's quite useless for you since CSS in Firefox works a tiny bit differently (variables, removing !important, replacing data uris, ...), but I'm providing so you can copy paste a few stuff in it.

Comment 36

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2: differentiate styling on Windows 8 Desktop

Now with updated toolbarbutton styles and I went through various toolkit components to make it look more integrated with Win8.

@ntim007: thanks trying to help out! Your userstyle was most useful as a sanity check for me (to see if forgot something). I prolly forgot something still, but that's why we have a review process, right Mike? ;)

Comment 37

[Peter Henkel [:Terepin]]

There is one more complete user style: http://userstyles.org/styles/88884/firefox-australis-mockup-theme-windows-8
However, this one has one bug with hover tab position on tabs. Bot other than that it's pretty close to mockup.

Comment 38

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #36)
> @ntim007: thanks trying to help out! Your userstyle was most useful as a
> sanity check for me (to see if forgot something). I prolly forgot something
> still, but that's why we have a review process, right Mike? ;)

Glad it helped ;) Without that review process, Firefox would be quite unstable right now.

>#PanelUI-menu-button::before {
>    content: "";
>    display: -moz-box;
>    background: linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0)),
>                linear-gradient(rgba(23,51,78,0), rgba(23,51,78,.3) 30%, rgba(23,51,78,.3) 70%, rgba(23,51,78,0)),
>               linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0));
>    width: 1px;
>    height: 32px;
>    margin: 0 3px;
>  }
Shouldn't this be applied in the default Australis theme (the seperator is visible on all platforms) ?

(In reply to Peter Henkel [:Terepin] from comment #37)
> There is one more complete user style:
> http://userstyles.org/styles/88884/firefox-australis-mockup-theme-windows-8
> However, this one has one bug with hover tab position on tabs. Bot other
> than that it's pretty close to mockup.

It doesn't seem to style the toolbars in the UX branch, but yeah, that one is more complete. :)

Comment 39

[Peter Henkel [:Terepin]]

Does this bug contain changes across entire FX, or just main windows? Because if not, ICUI will look inconsistent.

Comment 40

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

(In reply to Peter Henkel [:Terepin] from comment #39)
> Does this bug contain changes across entire FX, or just main windows?
> Because if not, ICUI will look inconsistent.

What is "ICUI"? I tried Googling it and searching UrbanDictionary but all I found was "ICU Medical, Inc."

Comment 41

[Peter Henkel [:Terepin]]

My bad, I get so used to using short names, heh. I meant In-Content UI.

Comment 42

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

This bug is only for the main window area, not touch in-content UI as far as I know.

In-content UI changes should be pushed to a separate bug to keep the code reviews here manageable. Please file a different bug and specify what in particular should be changed :)

Comment 43

[Peter Henkel [:Terepin]]

I will fill bug, but the changes will be up to Stephen.

Comment 44

[Dão Gottwald [:dao]]

(In reply to ntim007 from comment #38)
> >#PanelUI-menu-button::before {
> >    content: "";
> >    display: -moz-box;
> >    background: linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0)),
> >                linear-gradient(rgba(23,51,78,0), rgba(23,51,78,.3) 30%, rgba(23,51,78,.3) 70%, rgba(23,51,78,0)),
> >               linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0));
> >    width: 1px;
> >    height: 32px;
> >    margin: 0 3px;
> >  }
> Shouldn't this be applied in the default Australis theme (the seperator is
> visible on all platforms) ?

There's actually a separate bug filed on that, it doesn't belong in here.

Comment 45

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

(In reply to Dão Gottwald [:dao] from comment #44)
> (In reply to ntim007 from comment #38)
> > >#PanelUI-menu-button::before {
> > >    content: "";
> > >    display: -moz-box;
> > >    background: linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0)),
> > >                linear-gradient(rgba(23,51,78,0), rgba(23,51,78,.3) 30%, rgba(23,51,78,.3) 70%, rgba(23,51,78,0)),
> > >               linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0));
> > >    width: 1px;
> > >    height: 32px;
> > >    margin: 0 3px;
> > >  }
> > Shouldn't this be applied in the default Australis theme (the seperator is
> > visible on all platforms) ?
> 
> There's actually a separate bug filed on that, it doesn't belong in here.

Bug 870605.

Comment 46

[Dão Gottwald [:dao]]

Comment on attachment 786956 [details] [diff] [review]
Patch v2: differentiate styling on Windows 8 Desktop

>-@media (-moz-windows-default-theme) {
>+@media not (-moz-os-version: windows-win8) and (-moz-windows-default-theme) {

This doesn't work, see https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Media_queries#not and https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Media_queries#Pseudo-BNF_%28for_those_of_you_that_like_that_kind_of_thing%29

>   #main-window[sizemode="maximized"] #titlebar-buttonbox {
>     -moz-margin-end: 3px;
>   }
> 
>+
>   #main-window {
>     -moz-appearance: -moz-win-borderless-glass;
>     background: transparent;
>   }

Please drop this change.

>+    @media not (-moz-os-version: windows-win8)  {

This doesn't work either. (Also, nit: drop the extra space before '{').

>+  @media (-moz-windows-default-theme) {
>+    #nav-bar .toolbarbutton-1:not([disabled]):-moz-any(:hover,[open]) > .toolbarbutton-menubutton-button > .toolbarbutton-icon,
>+    #nav-bar .toolbarbutton-1:not([disabled]):hover > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon,
>+    #nav-bar .toolbarbutton-1:not([disabled]):not([checked]):not([open]):not(:active):hover > .toolbarbutton-icon,
>+    #nav-bar .toolbarbutton-1:not([disabled]):not([checked]):not([open]):not(:active):hover > .toolbarbutton-badge-container,
>+    @conditionalForwardWithUrlbar@ > .toolbarbutton-1:-moz-any([disabled],:not([open]):not([disabled]):not(:active)) > .toolbarbutton-icon {
>+      background-image: linear-gradient(hsla(220,1%,80%,.55), hsla(220,1%,80%,.52));
>+      border-color: hsla(220,1%,80%,.85) hsla(220,1%,80%,.8) hsla(220,1%,80%,.8);
>+      box-shadow: none;
>+    }
>+
>+    #nav-bar .toolbarbutton-1 > .toolbarbutton-menubutton-button:not([disabled]):hover:active > .toolbarbutton-icon,
>+    #nav-bar .toolbarbutton-1[open] > .toolbarbutton-menubutton-dropmarker:not([disabled]) > .dropmarker-icon,
>+    #nav-bar .toolbarbutton-1:not([disabled]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-icon,
>+    #nav-bar .toolbarbutton-1:not([disabled]):-moz-any([open],[checked],:hover:active) > .toolbarbutton-badge-container {
>+      background-image: linear-gradient(hsla(220,1%,80%,.28), hsla(220,1%,80%,.25));
>+      border-color: hsla(220,1%,80%,.60) hsla(220,1%,80%,.45) hsla(220,1%,80%,.45);
>+      background-color: none;
>+      box-shadow: 0 1px 1px hsla(220,1%,80%,.3) inset,
>+                  -1px 0 1px hsla(220,1%,80%,0) inset,
>+                  /* allows windows-keyhole-forward-clip-path to be used for non-hover as well as hover: */
>+                  0 1px 0 hsla(220,1%,80%,0),
>+                  0 0 2px hsla(220,1%,80%,0);
>+    }
>+  }

It will be tricky to keep this chunk up to date and working if we change the default styling in browser.css. Why do we need this? Can't see it in attachment 742384 [details].

>+  #PanelUI-menu-button::before {
>+    content: "";
>+    display: -moz-box;
>+    background: linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0)),
>+                linear-gradient(rgba(23,51,78,0), rgba(23,51,78,.3) 30%, rgba(23,51,78,.3) 70%, rgba(23,51,78,0)),
>+                linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0));
>+    width: 1px;
>+    height: 32px;
>+    margin: 0 3px;
>+  }

As mentioned in previous comments, this doesn't belong in this bug.

>--- a/browser/themes/windows/places/organizer-aero.css
>+++ b/browser/themes/windows/places/organizer-aero.css

>   #searchFilter {
>     -moz-appearance: none;
>     padding: 2px;
>     -moz-padding-start: 4px;
>     background-clip: padding-box;
>     border: 1px solid rgba(0,0,0,.32);
>     border-radius: 2px;
>   }
>+
>+  @media (-moz-os-version: windows-win8) {
>+    #searchFilter {
>+      border-radius: 0;
>+    }
>+  }
> }

  @media (-moz-os-version: windows-vista),
         (-moz-os-version: windows-win7) {
    #searchFilter {
      border-radius: 2px;
    }
  }

>--- a/toolkit/themes/windows/global/inContentUI-aero.css
>+++ b/toolkit/themes/windows/global/inContentUI-aero.css

> %define WINDOWS_AERO
> %include inContentUI.css
> %undef WINDOWS_AERO
>+
>+@media (-moz-os-version: windows-win8) {
>+  *|*.main-content,
>+  *|button,
>+  menulist,
>+  colorpicker[type="button"] {
>+    border-radius: 0;
>+  }
>+}

Same as above, and put this in inContentUI.css.

>--- a/toolkit/themes/windows/global/popup.css
>+++ b/toolkit/themes/windows/global/popup.css

>+@media (-moz-os-version: windows-win8)  {
>+  .panel-arrowcontent {
>+    border-radius: 0;
>+  }
>+}

There are places where panel content has a radius to adjust to the panel's border radius, e.g.: http://hg.mozilla.org/mozilla-central/annotate/fd4cf30428b0/browser/themes/windows/downloads/downloads.css#l37

>--- a/toolkit/themes/windows/global/toolbar.css
>+++ b/toolkit/themes/windows/global/toolbar.css
>@@ -53,16 +53,28 @@ toolbarseparator {
>   border-top: 2px solid transparent;
>   border-bottom: 2px solid transparent;
>   border-left: 3px solid transparent;
>   border-right: 3px solid transparent;
>   -moz-border-left-colors  : transparent transparent ThreeDShadow;
>   -moz-border-right-colors : transparent transparent ThreeDHighlight;
> }
> 
>+@media (-moz-os-version: windows-win8)  {
>+  toolbarseparator {
>+    -moz-appearance: none;
>+    background: linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0)),
>+                linear-gradient(rgba(23,51,78,0), rgba(23,51,78,.3) 30%, rgba(23,51,78,.3) 70%, rgba(23,51,78,0)),
>+                linear-gradient(rgba(255,255,255,0), rgba(255,255,255,.3) 30%, rgba(255,255,255,.3) 70%, rgba(255,255,255,0));
>+    width: 1px;
>+    margin: 0 3px;
>+    border: 0;
>+  }
>+}

Not sure why that would be part of this bug. Can you drop it?

Comment 47

[Mike de Boer [:mikedeboer]]

Dão, thanks for the review! I agree with all of your points and will address them in the next patch. I needed the rules for #nav-bar toolbarbutton states to look more Windows 8-ish.

There's one thing I'm feeling rather uneasy about: the colors I picked are from mockups provided and linked to from this bug, which are all low-res. Currently I'm at my limit of what I can extract from a design spec without layers to read color and transparency values from.

In others words, as far as implementing this design is concerned, I'm at my limit here, pending a more high-res and complete version.

Comment 48

[Mike de Boer [:mikedeboer]]

Stephen, apart from the thing mconley asked you, could you also tell me which colors the borders and button-face the buttons on the navbar should have - for each of the normal, hover and active states?

Thanks!

Comment 49

[Tim Nguyen :ntim]

http://people.mozilla.org/~shorlander/mockups-interactive/australis-interactive-mockups/windows8.html
The interactive mockup

Comment 50

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1: Windows 8 toolkit theme adjustments

Comment 51

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2: Windows 8 browser theme adjustments

I was thinking that this is going in the right direction, but you can never be too sure!
That's why I'm asking for feedback this round, Dão, because I'd like to know if I'm on the right track here and if I missed anything.

I didn't fully implement the new Downloads Panel design, because I think that belongs in a follow-up. I just changed the "Show All Downloads" button to be NOT aero-blue.

I'm curious what you think!

Comment 52

[Tim Nguyen :ntim]

I don't really understand the ifdef in the patches but I think your patches seem to ignore XP. :)

Comment 53

[Dão Gottwald [:dao]]

Comment on attachment 831565 [details] [diff] [review]
Patch 1: Windows 8 toolkit theme adjustments

>+/* Win8 and beyond. */
>+.panel-arrowcontent {
>+  padding: 4px;
>+  color: #222426;
>+  background: hsla(0,0%,100%,.97);
>+  background-clip: padding-box;
>+  border: 1px solid hsla(210,4%,10%,.2);

Not sure why you're hardcoding a specific color, background-color and border-color (and why the latter two have an alpha channel). We should probably keep using native colors such as -moz-dialogtext, -moz-dialog and threedshadow.

The overall approach seems reasonable, though.

Comment 54

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #53)
> Comment on attachment 831565 [details] [diff] [review]
> Patch 1: Windows 8 toolkit theme adjustments
> 
> >+/* Win8 and beyond. */
> >+.panel-arrowcontent {
> >+  padding: 4px;
> >+  color: #222426;
> >+  background: hsla(0,0%,100%,.97);
> >+  background-clip: padding-box;
> >+  border: 1px solid hsla(210,4%,10%,.2);
> 
> Not sure why you're hardcoding a specific color, background-color and
> border-color (and why the latter two have an alpha channel). We should
> probably keep using native colors such as -moz-dialogtext, -moz-dialog and
> threedshadow.
> 
> The overall approach seems reasonable, though.
These colors are from shorlander's mockup : http://people.mozilla.org/~shorlander/mockups-interactive/australis-interactive-mockups/windows8.html

Comment 55

[Stephen Horlander [:shorlander]]

(In reply to ntim007 from comment #54)
> (In reply to Dão Gottwald [:dao] from comment #53)
> > Comment on attachment 831565 [details] [diff] [review]
> > Patch 1: Windows 8 toolkit theme adjustments
> > 
> > >+/* Win8 and beyond. */
> > >+.panel-arrowcontent {
> > >+  padding: 4px;
> > >+  color: #222426;
> > >+  background: hsla(0,0%,100%,.97);
> > >+  background-clip: padding-box;
> > >+  border: 1px solid hsla(210,4%,10%,.2);
> > 
> > Not sure why you're hardcoding a specific color, background-color and
> > border-color (and why the latter two have an alpha channel). We should
> > probably keep using native colors such as -moz-dialogtext, -moz-dialog and
> > threedshadow.
> > 
> > The overall approach seems reasonable, though.
> These colors are from shorlander's mockup :
> http://people.mozilla.org/~shorlander/mockups-interactive/australis-
> interactive-mockups/windows8.html

Yes, it is intentional for the panel color to be consistent on all versions of Windows. So I don't think -moz-dialog will work.

Comment 56

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1.1: Windows 8 toolkit theme adjustments

Comment 57

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.1: Windows 8 browser theme adjustments

Comment 58

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.2: Windows 8 browser theme adjustments

forgot to remove some leftover popup.css changes

Comment 59

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.3: Windows 8 browser theme adjustments

updated bookmark-button styles and removed superfluous comments

Comment 60

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.4: Windows 8 browser theme adjustments

LW Theme support for the back-button. Might be worth considering using colors with alpha-factor throughout (also in non-lw-theme mode) for the back-button, because it really is the odd-one-out atm.

Comment 61

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.5: Windows 8 browser theme adjustments

* back-button style (bgcolor) now with generic hsla values.
* added panelUIOverlay-aero.css to allow forward-compatible media queries for panel UI updates

Comment 62

[Mike de Boer [:mikedeboer]]

ntim007, the patch that is currently attached & pending review does not rely on the `-moz-os-version: win8` media query. Therefore, this bug does not depend on bug 907373.

Thanks for keeping track!

Comment 63

[Dão Gottwald [:dao]]

Comment on attachment 8336923 [details] [diff] [review]
Patch 2.5: Windows 8 browser theme adjustments

not a full review, just the first issue I noticed:

>--- a/browser/themes/shared/identity-block.inc.css
>+++ b/browser/themes/shared/identity-block.inc.css

>+/* all OSes and Windows < 8 */

Why does Windows 8 need special treatment here while we can share one style across Linux, OS X and Windows <8? Since the difference between Windows 8 and Windows 7 seems smaller than, say, between Linux and OS X or Windows XP and OS X, this doesn't really make sense to me.

Comment 64

[Dão Gottwald [:dao]]

Comment on attachment 8336059 [details] [diff] [review]
Patch 1.1: Windows 8 toolkit theme adjustments

>+/* Win8 and beyond. */
>+.panel-arrowcontent {
>+  padding: 4px;
>+  color: #222426;
>+  background: hsla(0,0%,100%,.97);
>+  background-clip: padding-box;
>+  border: 1px solid hsla(210,4%,10%,.2);
>+  box-shadow: 0 3px 5px hsla(210,4%,10%,.1),
>+              0 0 7px hsla(210,4%,10%,.1);
>+  margin: 4px;
>+}

AFAIK, the 97% background opacity will downgrade us from sub-pixel to grayscale anti-aliasing for text rendering. We should avoid that, for aesthetic and legibility reasons.

Also, these hardcoded background and text colors aren't going to work out for high-contrast themes and possibly other OS themes. Can you use -moz-field and -moz-fieldText?

Comment 65

[Dão Gottwald [:dao]]

Comment on attachment 8336059 [details] [diff] [review]
Patch 1.1: Windows 8 toolkit theme adjustments

>+/* Win8 and beyond. */
>+.panel-arrowcontent {
>+  padding: 4px;

>+/* < Win8 */
> .panel-arrowcontent {
>   border-radius: 4px;
>   padding: 10px;

Why is the padding different? Can we unify this across Windows versions?

Comment 66

[Mike de Boer [:mikedeboer]]

> >+/* Win8 and beyond. */
> >+.panel-arrowcontent {
> >+  padding: 4px;
> >+  color: #222426;
> >+  background: hsla(0,0%,100%,.97);
> >+  background-clip: padding-box;
> >+  border: 1px solid hsla(210,4%,10%,.2);
> >+  box-shadow: 0 3px 5px hsla(210,4%,10%,.1),
> >+              0 0 7px hsla(210,4%,10%,.1);
> >+  margin: 4px;
> >+}
> 
> AFAIK, the 97% background opacity will downgrade us from sub-pixel to
> grayscale anti-aliasing for text rendering. We should avoid that, for
> aesthetic and legibility reasons.
> 
> Also, these hardcoded background and text colors aren't going to work out
> for high-contrast themes and possibly other OS themes. Can you use
> -moz-field and -moz-fieldText?

Stephen, I'm ok with changing the `background` and `color` values to `moz-field` and `moz-fieldText` respectively for panels, but are you? If not, what'd be your suggestion?

Comment 67

[Tim Nguyen :ntim]

Don't apply opacity to the url bar and search bar in your patch, because if the window is too small, the toolbar buttons will be visible underneath it (I tried using an userstyle).
You could use rgba instead, or wait until the bug where toolbar buttons are under the url bar is fixed.

Comment 68

[Justin Dolske [:Dolske]]

This seems to have stalled?

Let's go with Dao's suggestion in comment 64 to get this rolling. If Steven would like to tweak further, that can happen in a followup.

Is comment 65 the only other pending review comment? Let's just fix or followup that too.

Comment 69

[Mike de Boer [:mikedeboer]]

Justin, my bad that work here appears to be stalling; I went ahead with Dão's suggestion. I expect to have a fresh set of patches ready today.

Comment 70

[Mike de Boer [:mikedeboer]]

Okay, this *would* be today if I could compile m-c on Windows 8.1. I already regret upgrading.

Comment 71

[Stephen Horlander [:shorlander]]

(In reply to Justin Dolske [:Dolske] from comment #68)
> This seems to have stalled?
> 
> Let's go with Dao's suggestion in comment 64 to get this rolling. If Steven
> would like to tweak further, that can happen in a followup.
> 
> Is comment 65 the only other pending review comment? Let's just fix or
> followup that too.

I think those values should work here. Although I don't know that will always work. Do we have another way to specify high-contrast specific changes?

Comment 72

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1.2: Windows 8 toolkit theme adjustments

Comment 73

[Mike de Boer [:mikedeboer]]

(In reply to Tim Nguyen [:ntim] from comment #67)
> Don't apply opacity to the url bar and search bar in your patch, because if
> the window is too small, the toolbar buttons will be visible underneath it
> (I tried using an userstyle).
> You could use rgba instead, or wait until the bug where toolbar buttons are
> under the url bar is fixed.

Tim, how can you get buttons beneath the urlbar? In Australis we now have an overflow-menu, which consumes all the buttons as the window shrinks.

Comment 74

[Mike de Boer [:mikedeboer]]

(In reply to Dão Gottwald [:dao] from comment #63)
> Why does Windows 8 need special treatment here while we can share one style
> across Linux, OS X and Windows <8? Since the difference between Windows 8
> and Windows 7 seems smaller than, say, between Linux and OS X or Windows XP
> and OS X, this doesn't really make sense to me.

Well, the identity block separator is defined as a subtle gradient. It's a detail that Windows 8 doesn't do subtle, let alone gradients!

Comment 75

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

Comment on attachment 8347327 [details] [diff] [review]
Patch 1.2: Windows 8 toolkit theme adjustments

Over to a toolkit reviewer.

Comment 76

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #73)
> (In reply to Tim Nguyen [:ntim] from comment #67)
> > Don't apply opacity to the url bar and search bar in your patch, because if
> > the window is too small, the toolbar buttons will be visible underneath it
> > (I tried using an userstyle).
> > You could use rgba instead, or wait until the bug where toolbar buttons are
> > under the url bar is fixed.
> 
> Tim, how can you get buttons beneath the urlbar? In Australis we now have an
> overflow-menu, which consumes all the buttons as the window shrinks.
The "nooverflow" attribute items (social buttons,...) bleed through. Also, the forward button also bleeds through when hovering the back button.

Comment 77

[Dão Gottwald [:dao]]

Comment on attachment 8347327 [details] [diff] [review]
Patch 1.2: Windows 8 toolkit theme adjustments

>+++ b/toolkit/themes/windows/global/popup-aero.css
>@@ -0,0 +1,7 @@
>+/* This Source Code Form is subject to the terms of the Mozilla Public
>+ * License, v. 2.0. If a copy of the MPL was not distributed with this
>+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>+
>+%define WINDOWS_AERO
>+%include popup.css
>+%undef WINDOWS_AERO

This will produce a duplicate license header in the generated popup.css. Please use this style:

% This Source Code Form is subject to the terms of the Mozilla Public
% License, v. 2.0. If a copy of the MPL was not distributed with this
% file, You can obtain one at http://mozilla.org/MPL/2.0/.

>+/* Win8 and beyond. */

Please remove this comment, as some of those styles are used for all Windows versions.

> .panel-arrowcontent {
>-  border-radius: 4px;
>-  padding: 10px;
>-  color: -moz-DialogText;
>-  background: -moz-Dialog;
>+  padding: 4px;
>+  color: -moz-FieldText;
>+  background: -moz-field;
>   background-clip: padding-box;
>-  border: 1px solid ThreeDShadow;
>+  border: 1px solid hsla(210,4%,10%,.2);
>+  box-shadow: 0 3px 5px hsla(210,4%,10%,.1),
>+              0 0 7px hsla(210,4%,10%,.1);
>   margin: 4px;
> }

>+%ifdef WINDOWS_AERO
>+@media (-moz-os-version: windows-vista),
>+       (-moz-os-version: windows-win7) {
>+%endif
>+/* < Win8 */
>+.panel-arrowcontent {
>+  border-radius: 4px;
>+  color: -moz-DialogText;
>+  background: -moz-Dialog;

If I understand comment 55 correctly, you should use -moz-field for all versions of Windows.

> .panel-arrow[side="top"],
> .panel-arrow[side="bottom"] {
>-  list-style-image: url("chrome://global/skin/arrow/panelarrow-vertical.svg");
>+  list-style-image: url("chrome://global/skin/arrow/panelarrow-vertical-white.png");

Seems like we should get rid of panelarrow-*-white.png. You'll need to update the svg files to use -moz-field instead of -moz-dialog.

>--- a/toolkit/themes/windows/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/windows/mozapps/extensions/extensions.css
>@@ -294,21 +294,29 @@
>     width: 12em;
>   }
> }
> 
> @media (-moz-windows-default-theme) {
>   #header-search {
>     -moz-appearance: none;
>     border: 1px solid rgba(0, 0, 0, 0.32);
>-    border-radius: 2.5px;
>     padding-bottom: 2px;
>     background-color: rgba(255, 255, 255, 0.4);
>   }
>-
>+%ifdef WINDOWS_AERO
>+@media (-moz-os-version: windows-vista),
>+       (-moz-os-version: windows-win7) {
>+%endif
>+  #header-search {
>+    border-radius: 2.5px;
>+  }
>+%ifdef WINDOWS_AERO
>+}
>+%endif
>   #header-search:hover {
>     background-color: rgba(255, 255, 255, .75);
>   }

nit: add empty lines before and after the added stuff

Comment 78

[Dão Gottwald [:dao]]

Comment on attachment 8336923 [details] [diff] [review]
Patch 2.5: Windows 8 browser theme adjustments

(In reply to Mike de Boer [:mikedeboer] from comment #74)
> (In reply to Dão Gottwald [:dao] from comment #63)
> > Why does Windows 8 need special treatment here while we can share one style
> > across Linux, OS X and Windows <8? Since the difference between Windows 8
> > and Windows 7 seems smaller than, say, between Linux and OS X or Windows XP
> > and OS X, this doesn't really make sense to me.
> 
> Well, the identity block separator is defined as a subtle gradient. It's a
> detail that Windows 8 doesn't do subtle, let alone gradients!

This just seems to be an arbitrary difference in the mockup. If it is an intentional change, I don't see why we wouldn't adopt it cross-platform.

>+/* Win8 and beyond. */
>+@media not all and (-moz-os-version: windows-vista) {
>+  @media not all and (-moz-os-version: windows-win7) {
>+    #main-window[sizemode=normal] #nav-bar {
>+      border-top-left-radius: 0;
>+      border-top-right-radius: 0;
>+    }
>+
>+    #urlbar,
>+    .searchbar-textbox,
>+    #main-menubar:not(:-moz-lwtheme):not(:-moz-window-inactive) {
>+      border-radius: 0;
>+    }

How about doing it like in the toolkit patch, i.e. setting the radii only where you want them instead resetting them for Win 8?

>+    #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);
>+      opacity: .8;
>+      transition-property: margin-left, opacity, border-color, background-color;
>+      transition-duration: 200ms;
>+    }

I'm afraid the reduced opacity again happens to be a detail where the mockup differs from our current styling, but not specifically to match Windows 8. So if we want this, then probably for all Windows versions or at least Windows 7 and beyond. Note that this would basically undo bug 692537, which seems backwards to me.

>+    /* 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);
>+    }

It's not immediately clear to me why this is Win8-specific.

>+    /* Introducing a checked/ panel opened state */
>+    #nav-bar .toolbarbutton-1:not([disabled]):-moz-any([open],[checked]) > .toolbarbutton-icon,
>+    #nav-bar .toolbarbutton-1:not([disabled]):-moz-any([open],[checked]) > .toolbarbutton-text,
>+    #nav-bar .toolbarbutton-1:not([disabled]):-moz-any([open],[checked]) > .toolbarbutton-badge-container,
>+    #nav-bar .toolbarbutton-1[open] > .toolbarbutton-menubutton-dropmarker:not([disabled]) > .dropmarker-icon {
>+      background-color: #177ee5;
>+      box-shadow: 0 1px 0 0 hsla(210,80%,20%,.1) inset,
>+                  0 0 0 1px hsla(210,80%,20%,.1),
>+                  0 0 0 1px hsla(210,4%,10%,.25);
>+    }

Are you sure you want this for checked buttons? A button being checked isn't necessarily a transient state. With that in mind, the blue background seems rather jarring.

>+    /* Introducing additional Back-button states: */
>+    #back-button:not([disabled="true"]):not([open="true"]):hover:active > .toolbarbutton-icon {
>+      background-color: hsla(210,4%,10%,.2) !important;
>+      box-shadow: 0 1px 0 0 hsla(210,4%,10%,.1) inset !important;
>+    }

Not sure what this is exactly doing. Is there a good reason for it to be Win8-specific?

>+    #back-button[open="true"]:not(:active):hover > .toolbarbutton-icon {
>+      background-color: #0f5499 !important;
>+    }

Where did you get this from? I don't see it in the mockup, or maybe I'm not looking at the right mockup?

Stopping here, as I suspect that remaining parts of the patch will raise some of the above questions again.

Comment 79

[Tim Nguyen :ntim]

(In reply to Dão Gottwald [:dao] from comment #78)
> >+    #back-button[open="true"]:not(:active):hover > .toolbarbutton-icon {
> >+      background-color: #0f5499 !important;
> >+    }
> 
> Where did you get this from? I don't see it in the mockup, or maybe I'm not
> looking at the right mockup?
All toolbar buttons open states should have a blue background. So I guess the back button should be affected for consistent.

Comment 80

[Tim Nguyen :ntim]

(In reply to Tim Nguyen [:ntim] from comment #79)
> (In reply to Dão Gottwald [:dao] from comment #78)
> > >+    #back-button[open="true"]:not(:active):hover > .toolbarbutton-icon {
> > >+      background-color: #0f5499 !important;
> > >+    }
> > 
> > Where did you get this from? I don't see it in the mockup, or maybe I'm not
> > looking at the right mockup?
> All toolbar buttons open states should have a blue background. So I guess
> the back button should be affected for consistent.

*in the mockup.

Comment 81

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1.3: Windows 8 toolkit theme adjustments

Comment 82

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.6: Windows 8 browser theme adjustments

Comment 83

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.7: Windows 8 browser theme adjustments

Forgot to yank out the identity-block changes. I deemed it out-of-scope for this bug, thus not worth debating; the mockup isn't decisive on the matter.

Comment 84

[Dão Gottwald [:dao]]

Comment on attachment 8348006 [details] [diff] [review]
Patch 1.3: Windows 8 toolkit theme adjustments

> <svg xmlns="http://www.w3.org/2000/svg"
>      width="10"
>      height="20">
>   <path d="M 10,0 L 0,10 10,20 z"
>-        fill="ThreeDShadow"/>
>+        fill="hsla(210,4%,10%,.2)"/>

> .panel-arrowcontent {
>-  border-radius: 4px;
>-  padding: 10px;
>-  color: -moz-DialogText;
>-  background: -moz-Dialog;
>+  padding: 4px;
>+  color: -moz-FieldText;
>+  background: -moz-field;
>   background-clip: padding-box;
>-  border: 1px solid ThreeDShadow;
>+  border: 1px solid hsla(210,4%,10%,.2);
>+  box-shadow: 0 3px 5px hsla(210,4%,10%,.1),
>+              0 0 7px hsla(210,4%,10%,.1);
>   margin: 4px;
> }

Have you tested how unconditionally changing the border from ThreeDShadow to hsla(210,4%,10%,.2) affects dark OS themes, like dark high-contrast themes? Seems like it would make the popup visually borderless, doesn't it?

>+.panel-arrow[side="left"],
>+.panel-arrow[side="right"] {
>+  list-style-image: url("chrome://global/skin/arrow/panelarrow-horizontal.svg");
>+  position: relative;
>+  margin-top: 10px;
>+  margin-bottom: 10px;
>+}

>-.panel-arrow[side="left"],
>-.panel-arrow[side="right"] {
>-  list-style-image: url("chrome://global/skin/arrow/panelarrow-horizontal.svg");
>-  position: relative;
>-  margin-top: 10px;
>-  margin-bottom: 10px;
>-}

Why are you moving this around?

> %ifdef XP_WIN
> @media (-moz-windows-default-theme) {
>-  .panel-arrowcontent {
>-    border-color: rgba(0,0,0,.3);
>-    box-shadow: 0 0 4px rgba(0,0,0,.3);
>-  }
>-
>   .panel-arrowcontent[side="top"] {
>     background-image: linear-gradient(white 1px, rgba(255,255,255,0) 15px);
>   }
> 
>   .panel-arrowcontent[side="bottom"] {
>     background-image: linear-gradient(to top, white 1px, rgba(255,255,255,0) 15px);
>   }
> 
>   .panel-arrowcontent[side="left"] {
>     background-image: linear-gradient(to right, white 1px, rgba(255,255,255,0) 15px);
>   }
> 
>   .panel-arrowcontent[side="right"] {
>     background-image: linear-gradient(to left, white 1px, rgba(255,255,255,0) 15px);
>   }
>-
>-  .panel-arrow[side="top"],
>-  .panel-arrow[side="bottom"] {
>-    list-style-image: url("chrome://global/skin/arrow/panelarrow-vertical-white.png");
>-  }
>-
>-  .panel-arrow[side="left"],
>-  .panel-arrow[side="right"] {
>-    list-style-image: url("chrome://global/skin/arrow/panelarrow-horizontal-white.png");
>-  }
> }
> %endif

The background-image stuff should go away too, shouldn't it?

Comment 85

[Peter Henkel [:Terepin]]

Attachment: http://goo.gl/I1hbh3

This person has done outstanding job of recreating Stephen's mockup and it went further than this bug (ICUI, other windows...).

Comment 86

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

(In reply to Peter Henkel [:Terepin] from comment #85)
> Created attachment 8355514 [details]
> http://goo.gl/I1hbh3
> 
> This person has done outstanding job of recreating Stephen's mockup and it
> went further than this bug (ICUI, other windows...).

This bug hasn't landed yet, and there's no compatible license for the link you sent, so we can't use it.

Comment 87

[Peter Henkel [:Terepin]]

I was just pint-pointing to it, that is all. :) And I was not aware that CSS code require a special license. I thought a simple approval from creator would suffice.

Comment 88

[Stanzilla]

(In reply to :Gijs Kruitbosch from comment #86)
> (In reply to Peter Henkel [:Terepin] from comment #85)
> > Created attachment 8355514 [details]
> > http://goo.gl/I1hbh3
> > 
> > This person has done outstanding job of recreating Stephen's mockup and it
> > went further than this bug (ICUI, other windows...).
> 
> This bug hasn't landed yet, and there's no compatible license for the link
> you sent, so we can't use it.

:Terepin: Nice!

I bet if someone from Mozilla contacted the guy, he would be more than happy to contribute his code to the project. But I guess this won't happen and Nightly will look unpolished on Win8 for months. Sad but true.

Comment 89

[Mike de Boer [:mikedeboer]]

Terepin, Stanzilla: this userstyle looks outstanding indeed. It looks like the author, blink, put a lot of time and effort into it!

However, I'm afraid it's not as simple as copy-pasting the CSS into mozilla-central sources and that'd it... If Windows 8 would've been the only Windows version that we target with the same theme, we could simply take the style rules and be done with it.

I could go and enumerate all the UI parts and combinations of firefox & OS settings, but for now I'd just like to ensure you that the work remaining for this bug is my no. 1 priority at this time.

Comment 90

[u428464]

Some parts aren't Win 8 specific though. AFAIK the panels are too be consistent on every platform for example.

Comment 91

[Peter Henkel [:Terepin]]

(In reply to Mike de Boer [:mikedeboer] from comment #89)
> Terepin, Stanzilla: this userstyle looks outstanding indeed. It looks like
> the author, blink, put a lot of time and effort into it!
> 
> However, I'm afraid it's not as simple as copy-pasting the CSS into
> mozilla-central sources and that'd it... If Windows 8 would've been the only
> Windows version that we target with the same theme, we could simply take the
> style rules and be done with it.
> 
> I could go and enumerate all the UI parts and combinations of firefox & OS
> settings, but for now I'd just like to ensure you that the work remaining
> for this bug is my no. 1 priority at this time.

Oh, don't get my wrong, I wasn't implying that you're doing bad work or something. I just wanted to help out at least a bit. I know making patch is far more complicated than creating user style, but I thought you could use at least something.

Comment 92

[Mike de Boer [:mikedeboer]]

(In reply to Peter Henkel [:Terepin] from comment #91)
> Oh, don't get my wrong, I wasn't implying that you're doing bad work or
> something. I just wanted to help out at least a bit. I know making patch is
> far more complicated than creating user style, but I thought you could use
> at least something.

I didn't get you wrong! :) Thanks for the help!

Comment 93

[Stanzilla]

(In reply to Peter Henkel [:Terepin] from comment #91)
> (In reply to Mike de Boer [:mikedeboer] from comment #89)
> > Terepin, Stanzilla: this userstyle looks outstanding indeed. It looks like
> > the author, blink, put a lot of time and effort into it!
> > 
> > However, I'm afraid it's not as simple as copy-pasting the CSS into
> > mozilla-central sources and that'd it... If Windows 8 would've been the only
> > Windows version that we target with the same theme, we could simply take the
> > style rules and be done with it.
> > 
> > I could go and enumerate all the UI parts and combinations of firefox & OS
> > settings, but for now I'd just like to ensure you that the work remaining
> > for this bug is my no. 1 priority at this time.
> 
> Oh, don't get my wrong, I wasn't implying that you're doing bad work or
> something. I just wanted to help out at least a bit. I know making patch is
> far more complicated than creating user style, but I thought you could use
> at least something.

Same here, I really know how complicated the Firefox CSS is. I just meant maybe the guy is interested in helping out, not just copy/pasting his stuff hin.

Comment 94

[Mike de Boer [:mikedeboer]]

(In reply to Dão Gottwald [:dao] from comment #84)
> Have you tested how unconditionally changing the border from ThreeDShadow to
> hsla(210,4%,10%,.2) affects dark OS themes, like dark high-contrast themes?
> Seems like it would make the popup visually borderless, doesn't it?

Dão, what do you think the condition should be? Aero-only? Win8+ only?

Comment 95

[Dão Gottwald [:dao]]

(In reply to Mike de Boer [:mikedeboer] from comment #94)
> (In reply to Dão Gottwald [:dao] from comment #84)
> > Have you tested how unconditionally changing the border from ThreeDShadow to
> > hsla(210,4%,10%,.2) affects dark OS themes, like dark high-contrast themes?
> > Seems like it would make the popup visually borderless, doesn't it?
> 
> Dão, what do you think the condition should be? Aero-only? Win8+ only?

-moz-windows-default-theme should work here regardless of the Windows version.

Comment 96

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1.4: Windows 8 toolkit theme adjustments

Comment 97

[Dão Gottwald [:dao]]

Comment on attachment 8357780 [details] [diff] [review]
Patch 1.4: Windows 8 toolkit theme adjustments

>-%ifdef XP_WIN
> @media (-moz-windows-default-theme) {

Why did you remove the ifdef? Note that this is built on non-Windows platforms, contrary to what the file path might suggest.

>   .panel-arrowcontent {
>-    border-color: rgba(0,0,0,.3);
>-    box-shadow: 0 0 4px rgba(0,0,0,.3);
>-  }
>-
>-  .panel-arrowcontent[side="top"] {
>-    background-image: linear-gradient(white 1px, rgba(255,255,255,0) 15px);
>-  }
>-
>-  .panel-arrowcontent[side="bottom"] {
>-    background-image: linear-gradient(to top, white 1px, rgba(255,255,255,0) 15px);
>-  }
>-
>-  .panel-arrowcontent[side="left"] {
>-    background-image: linear-gradient(to right, white 1px, rgba(255,255,255,0) 15px);
>-  }
>-
>-  .panel-arrowcontent[side="right"] {
>-    background-image: linear-gradient(to left, white 1px, rgba(255,255,255,0) 15px);
>+    border: 1px solid hsla(210,4%,10%,.2);
>+    box-shadow: 0 3px 5px hsla(210,4%,10%,.1),
>+                0 0 7px hsla(210,4%,10%,.1);
>   }

Please use border-color instead of the border shorthand.

Comment 98

[Mike de Boer [:mikedeboer]]

Attachment: Patch 1.5: Windows 8 toolkit theme adjustments

Comment 99

[Mike de Boer [:mikedeboer]]

Attachment: Patch 2.8: Windows 8 browser theme adjustments

Now with new icons from the ZIP file attached to bug 897268.

Comment 100

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #99)
> Created attachment 8358395 [details] [diff] [review]
> Patch 2.8: Windows 8 browser theme adjustments
> 
> Now with new icons from the ZIP file attached to bug 897268.

I'd name the image files to something more like : Toolbar-win8.png and Toolbar-inverted-win8.png

Comment 101

[Tim Nguyen :ntim]

Btw, you should also adjust the hover effects, as they use the 2nd row of those images instead of the first row. I guess you could copy the code from OSX files. Or that could happen in a follow up bug.

Comment 102

[Mike de Boer [:mikedeboer]]

(In reply to Tim Nguyen [:ntim] from comment #100)
> I'd name the image files to something more like : Toolbar-win8.png and
> Toolbar-inverted-win8.png

I considered that but decided differently, because the icons will also work for Windows 8.1, 9 and further.

Comment 103

[Tim Nguyen :ntim]

(In reply to Mike de Boer [:mikedeboer] from comment #102)
> (In reply to Tim Nguyen [:ntim] from comment #100)
> > I'd name the image files to something more like : Toolbar-win8.png and
> > Toolbar-inverted-win8.png
> 
> I considered that but decided differently, because the icons will also work
> for Windows 8.1, 9 and further.

Toolbar-modern and Toolbar-inverted-modern ?

Comment 104

[Tim Nguyen :ntim]

Oh and btw, have you considered adding support for future windows versions ? Because the media query -moz-os-version:windows-win8 will need to be updated with the next windows release. You'll have to use -moz-os-version:windows-win8 or -moz-os-version:windows-win9.... 

So I guess you should file a new bug about targetting versions higher than windows 8. I've heard there's some kind of metro capable css preprocessor in Firefox, but I'm not sure.

Comment 105

[Mike de Boer [:mikedeboer]]

(In reply to Tim Nguyen [:ntim] from comment #104)
> Oh and btw, have you considered adding support for future windows versions ?
> Because the media query -moz-os-version:windows-win8 will need to be updated
> with the next windows release. You'll have to use
> -moz-os-version:windows-win8 or -moz-os-version:windows-win9....

This patch is also about future Windows versions. I'm specifically not using the `-moz-os-version` query to avoid being not-forward-compatible like that.
So for this bug, support for `-moz-os-version:windows-winX` is not required.

Comment 106

[Masatoshi Kimura [:emk]]

We can't predict what random changes Microsoft will apply to the theme as they have done for Win8. We will have to update for each future Windows version anyway.

Comment 107

[Dão Gottwald [:dao]]

Mike, does the toolkit patch need any browser changes? If not, you might want to land while I review the browser patch. In fact, it might make sense to move this bug to toolkit, resolve it, and file a separate one for the browser changes.

Comment 108

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

Although I feel bad adding more into this bug, we got updated panel/palette glyphs for the "bookmarks toolbar items" placeholder widget thingummywhatsit in bug 922834. I've left the win8 part out of that patch because it made more sense to me to integrate it here. That's a separate image file, so it could also be a followup. Not the highest prio either (The aero icon will look non-terrible in the meantime).

Comment 109

[Mike de Boer [:mikedeboer]]

remote: https://hg.mozilla.org/mozilla-central/rev/9c3ddd178154

Comment 110

[Ryan VanderMeulen [:RyanVM]]

Backed out for Windows mochitest-other failures.
https://hg.mozilla.org/mozilla-central/rev/ad23b7d49a60

https://tbpl.mozilla.org/php/getParsedLog.php?id=33114870&tree=Mozilla-Central

Comment 111

[Tim Nguyen :ntim]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #110)
> Backed out for Windows mochitest-other failures.
> https://hg.mozilla.org/mozilla-central/rev/ad23b7d49a60
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=33114870&tree=Mozilla-
> Central

Restoring "padding:10px" instead of 4px on ".panel-arrowcontent" should be fixing the issue.

Comment 112

[Mike de Boer [:mikedeboer]]

Re-landed without padding change: https://hg.mozilla.org/integration/fx-team/rev/ecb27777014d

Comment 113

[u428464]

(In reply to Mike de Boer [:mikedeboer] from comment #112)
> Re-landed without padding change:
> https://hg.mozilla.org/integration/fx-team/rev/ecb27777014d

This padding is wanted on every version of Windows though.

Comment 114

[Mike de Boer [:mikedeboer]]

(In reply to Guillaume C. [:ge3k0s] from comment #113)
> This padding is wanted on every version of Windows though.

I know, will file another bug to re-add the padding with test updates (as per discussion with Dão).

Comment 115

[Mike de Boer [:mikedeboer]]

(In reply to Mike de Boer [:mikedeboer] from comment #114)
> I know, will file another bug to re-add the padding with test updates (as
> per discussion with Dão).

Filed as bug 961727.

Comment 116

[Phil Ringnalda (:philor)]

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

Comment 117

[Jacek Caban]

I landed a trivial fixup changing encoding (DOS encoding was causing problems with include when cross compiling):

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

Comment 118

[Carsten Book [:Tomcat]]

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

Comment 184

[Bogdan Maris, Desktop Test Engineering]

Tested on Windows 8 32/64bit, did some exploratory testing around and the changes of menus, buttons, color, hover effect look as intended.
http://people.mozilla.org/~shorlander/mockups-interactive/australis-interactive-mockups/windows8.html

Comment 185

[henryfhchan]

For menubuttons and the star/bookmarks menu button, the border (or box-shadow) of the buttons fade out at different intervals.  It seems the border of the left button transitions out nicely while that on the right is always sharp.

Hovering and unhovering over the buttons repeatedly (about after 10 or so times) seem to reduce the effect.

Comment 186

[Tim Nguyen :ntim]

(In reply to henryfhchan from comment #185)
> For menubuttons and the star/bookmarks menu button, the border (or
> box-shadow) of the buttons fade out at different intervals.  It seems the
> border of the left button transitions out nicely while that on the right is
> always sharp.
> 
> Hovering and unhovering over the buttons repeatedly (about after 10 or so
> times) seem to reduce the effect.

Please file a bug about this.

Comment 187

[henryfhchan]

Filed bug https://bugzilla.mozilla.org/show_bug.cgi?id=989660