Last Comment Bug 859751 - Theme adjustments for Windows 8
: Theme adjustments for Windows 8
Status: VERIFIED FIXED
[Australis:M?][Australis:P2]
:
Product: Toolkit
Classification: Components
Component: Themes (show other bugs)
: Trunk
: All Windows 8
-- normal with 4 votes (vote)
: mozilla29
Assigned To: Mike de Boer [:mikedeboer]
: Cornel Ionce [QA] (:cornel_ionce)
: Dão Gottwald [:dao]
Mentors:
http://people.mozilla.org/~shorlander...
Depends on: 878065 962083 962150 962395
Blocks: australis 940393 960517 961727
  Show dependency treegraph
 
Reported: 2013-04-09 04:29 PDT by Peter Henkel [:Terepin]
Modified: 2014-03-29 13:37 PDT (History)
39 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Firefox navbar vs Windows 8 Explorer (312.04 KB, image/png)
2013-04-09 04:29 PDT, Peter Henkel [:Terepin]
no flags Details
Australis - Windows 8 - i01 (254.78 KB, image/png)
2013-04-26 08:24 PDT, Stephen Horlander [:shorlander]
no flags Details
WIP Patch: differentiate styling on Win8 (11.68 KB, patch)
2013-08-01 10:25 PDT, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch v1: differentiate styling on Windows 8 Desktop (3.40 KB, patch)
2013-08-06 07:39 PDT, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
hover.png (2.88 KB, image/png)
2013-08-06 10:07 PDT, Peter Henkel [:Terepin]
no flags Details
pressed.png (2.88 KB, image/png)
2013-08-06 10:08 PDT, Peter Henkel [:Terepin]
no flags Details
Screenshot of userstyle matching 80% of shorlander's mockup (360.68 KB, image/png)
2013-08-06 15:15 PDT, Tim Nguyen :ntim
no flags Details
Userstyle matching 80% of shorlander's mockup (148.23 KB, text/plain)
2013-08-06 15:24 PDT, Tim Nguyen :ntim
no flags Details
Patch v2: differentiate styling on Windows 8 Desktop (10.83 KB, patch)
2013-08-07 08:35 PDT, Mike de Boer [:mikedeboer]
dao+bmo: review-
Details | Diff | Splinter Review
Patch 1: Windows 8 toolkit theme adjustments (6.10 KB, patch)
2013-11-13 08:27 PST, Mike de Boer [:mikedeboer]
dao+bmo: feedback-
Details | Diff | Splinter Review
Patch 2: Windows 8 browser theme adjustments (30.64 KB, patch)
2013-11-13 08:30 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch 1.1: Windows 8 toolkit theme adjustments (6.10 KB, patch)
2013-11-21 06:47 PST, Mike de Boer [:mikedeboer]
dao+bmo: review-
Details | Diff | Splinter Review
Patch 2.1: Windows 8 browser theme adjustments (30.76 KB, patch)
2013-11-21 06:47 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch 2.2: Windows 8 browser theme adjustments (29.85 KB, patch)
2013-11-21 06:54 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch 2.3: Windows 8 browser theme adjustments (30.12 KB, patch)
2013-11-22 05:27 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch 2.4: Windows 8 browser theme adjustments (30.97 KB, patch)
2013-11-22 09:44 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch 2.5: Windows 8 browser theme adjustments (32.15 KB, patch)
2013-11-22 10:44 PST, Mike de Boer [:mikedeboer]
dao+bmo: review-
Details | Diff | Splinter Review
Patch 1.2: Windows 8 toolkit theme adjustments (6.17 KB, patch)
2013-12-13 09:39 PST, Mike de Boer [:mikedeboer]
dao+bmo: review-
Details | Diff | Splinter Review
Patch 1.3: Windows 8 toolkit theme adjustments (12.64 KB, patch)
2013-12-16 04:45 PST, Mike de Boer [:mikedeboer]
dao+bmo: review-
Details | Diff | Splinter Review
Patch 2.6: Windows 8 browser theme adjustments (32.78 KB, patch)
2013-12-17 07:21 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch 2.7: Windows 8 browser theme adjustments (30.11 KB, patch)
2013-12-17 07:28 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
http://goo.gl/I1hbh3 (215.15 KB, image/png)
2014-01-03 04:32 PST, Peter Henkel [:Terepin]
no flags Details
Patch 1.4: Windows 8 toolkit theme adjustments (14.75 KB, patch)
2014-01-09 07:27 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review
Patch 1.5: Windows 8 toolkit theme adjustments (14.73 KB, patch)
2014-01-09 07:38 PST, Mike de Boer [:mikedeboer]
dao+bmo: review+
Details | Diff | Splinter Review
Patch 2.8: Windows 8 browser theme adjustments (83.65 KB, patch)
2014-01-10 06:55 PST, Mike de Boer [:mikedeboer]
no flags Details | Diff | Splinter Review

Description User image Peter Henkel [:Terepin] 2013-04-09 04:29:37 PDT
Created attachment 735108 [details]
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 User image Stephen Horlander [:shorlander] 2013-04-26 08:24:57 PDT
Created attachment 742384 [details]
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 User image Peter Henkel [:Terepin] 2013-04-27 07:31:53 PDT
Thanks Stephen! One thing though: The close button is actually red the whole time, just like in Win7.
Comment 3 User image Jim Mathies [:jimm] 2013-05-17 11:28:15 PDT
Is the fx button going away?
Comment 4 User image Matthew N. [:MattN] (PM if requests are blocking you) 2013-05-17 11:31:17 PDT
(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 User image Mike Conley (:mconley) 2013-06-06 10:09:14 PDT
Reporter - are there still problems with Australis on Windows 8?
Comment 6 User image Mike Conley (:mconley) 2013-06-06 10:09:46 PDT
Assigning to self if this is still an issue.
Comment 7 User image Guillaume C. [:ge3k0s] 2013-06-06 10:12:54 PDT
(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 User image Peter Henkel [:Terepin] 2013-06-06 11:34:17 PDT
(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 User image Peter Henkel [:Terepin] 2013-06-07 05:38:50 PDT
I have one technical question, if I may: Why is that many hover effects are hardcoded and not using system's styling?
Comment 10 User image Mike Conley (:mconley) 2013-06-19 11:48:24 PDT
Removing the items from M7 that do not block us from landing on m-c.
Comment 11 User image Tim Nguyen :ntim 2013-07-15 16:35:21 PDT
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 User image Mike de Boer [:mikedeboer] 2013-08-01 03:12:53 PDT
Mike, is it OK if I steal this from you?
Comment 13 User image Mike Conley (:mconley) 2013-08-01 08:18:55 PDT
(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 User image Mike de Boer [:mikedeboer] 2013-08-01 10:25:32 PDT
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?
Comment 15 User image Dão Gottwald [:dao] 2013-08-01 10:33:28 PDT
(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 User image Mike de Boer [:mikedeboer] 2013-08-01 10:48:08 PDT
(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 User image Peter Henkel [:Terepin] 2013-08-01 10:58:55 PDT
Can't we just follow Stephen's mockup?
Comment 18 User image Mike de Boer [:mikedeboer] 2013-08-01 11:05:37 PDT
(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 User image Peter Henkel [:Terepin] 2013-08-01 11:18:43 PDT
I'm asking because Stephen himself wrote "Use -moz-dialog and not the custom blue".
Comment 20 User image Tim Nguyen :ntim 2013-08-01 13:03:16 PDT
(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 User image Tim Nguyen :ntim 2013-08-01 13:05:29 PDT
- You could also remove border-radius from panels too
Comment 22 User image Matthew N. [:MattN] (PM if requests are blocking you) 2013-08-01 13:28:27 PDT
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 User image Mike de Boer [:mikedeboer] 2013-08-02 06:58:40 PDT
(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 User image Mike de Boer [:mikedeboer] 2013-08-06 07:39:51 PDT
Created attachment 786292 [details] [diff] [review]
Patch v1: differentiate styling on Windows 8 Desktop
Comment 25 User image Mike Conley (:mconley) 2013-08-06 08:37:14 PDT
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 User image Mike de Boer [:mikedeboer] 2013-08-06 09:02:37 PDT
(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 User image Dão Gottwald [:dao] 2013-08-06 09:49:28 PDT
(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 User image Peter Henkel [:Terepin] 2013-08-06 10:07:51 PDT
Created attachment 786360 [details]
hover.png
Comment 29 User image Peter Henkel [:Terepin] 2013-08-06 10:08:10 PDT
Created attachment 786362 [details]
pressed.png
Comment 30 User image Mike Conley (:mconley) 2013-08-06 10:38:26 PDT
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 User image Matthew N. [:MattN] (PM if requests are blocking you) 2013-08-06 11:18:35 PDT
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 User image Dão Gottwald [:dao] 2013-08-06 11:44:01 PDT
(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 User image Tim Nguyen :ntim 2013-08-06 14:18:23 PDT
(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 User image Tim Nguyen :ntim 2013-08-06 15:15:38 PDT
Created attachment 786541 [details]
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 User image Tim Nguyen :ntim 2013-08-06 15:24:19 PDT
Created attachment 786547 [details]
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 User image Mike de Boer [:mikedeboer] 2013-08-07 08:35:44 PDT
Created attachment 786956 [details] [diff] [review]
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 User image Peter Henkel [:Terepin] 2013-08-07 08:38:50 PDT
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 User image Tim Nguyen :ntim 2013-08-07 11:48:16 PDT
(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 User image Peter Henkel [:Terepin] 2013-08-07 11:55:23 PDT
Does this bug contain changes across entire FX, or just main windows? Because if not, ICUI will look inconsistent.
Comment 40 User image Jared Wein [:jaws] (please needinfo? me) 2013-08-07 12:04:09 PDT
(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 User image Peter Henkel [:Terepin] 2013-08-07 12:05:55 PDT
My bad, I get so used to using short names, heh. I meant In-Content UI.
Comment 42 User image Jared Wein [:jaws] (please needinfo? me) 2013-08-07 12:24:12 PDT
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 User image Peter Henkel [:Terepin] 2013-08-07 12:45:17 PDT
I will fill bug, but the changes will be up to Stephen.
Comment 44 User image Dão Gottwald [:dao] 2013-08-07 13:34:45 PDT
(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 User image Jared Wein [:jaws] (please needinfo? me) 2013-08-07 13:39:44 PDT
(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 User image Dão Gottwald [:dao] 2013-08-08 01:59:39 PDT
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 User image Mike de Boer [:mikedeboer] 2013-08-08 03:01:52 PDT
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 User image Mike de Boer [:mikedeboer] 2013-08-12 05:26:50 PDT
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 50 User image Mike de Boer [:mikedeboer] 2013-11-13 08:27:21 PST
Created attachment 831565 [details] [diff] [review]
Patch 1: Windows 8 toolkit theme adjustments
Comment 51 User image Mike de Boer [:mikedeboer] 2013-11-13 08:30:30 PST
Created attachment 831568 [details] [diff] [review]
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 User image Tim Nguyen :ntim 2013-11-13 12:30:05 PST
I don't really understand the ifdef in the patches but I think your patches seem to ignore XP. :)
Comment 53 User image Dão Gottwald [:dao] 2013-11-13 12:30:07 PST
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 User image Tim Nguyen :ntim 2013-11-13 12:33:11 PST
(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 User image Stephen Horlander [:shorlander] 2013-11-13 12:46:06 PST
(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 User image Mike de Boer [:mikedeboer] 2013-11-21 06:47:12 PST
Created attachment 8336059 [details] [diff] [review]
Patch 1.1: Windows 8 toolkit theme adjustments
Comment 57 User image Mike de Boer [:mikedeboer] 2013-11-21 06:47:57 PST
Created attachment 8336060 [details] [diff] [review]
Patch 2.1: Windows 8 browser theme adjustments
Comment 58 User image Mike de Boer [:mikedeboer] 2013-11-21 06:54:46 PST
Created attachment 8336071 [details] [diff] [review]
Patch 2.2: Windows 8 browser theme adjustments

forgot to remove some leftover popup.css changes
Comment 59 User image Mike de Boer [:mikedeboer] 2013-11-22 05:27:46 PST
Created attachment 8336761 [details] [diff] [review]
Patch 2.3: Windows 8 browser theme adjustments

updated bookmark-button styles and removed superfluous comments
Comment 60 User image Mike de Boer [:mikedeboer] 2013-11-22 09:44:18 PST
Created attachment 8336874 [details] [diff] [review]
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 User image Mike de Boer [:mikedeboer] 2013-11-22 10:44:36 PST
Created attachment 8336923 [details] [diff] [review]
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 User image Mike de Boer [:mikedeboer] 2013-11-24 08:38:46 PST
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 User image Dão Gottwald [:dao] 2013-11-28 08:42:02 PST
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 User image Dão Gottwald [:dao] 2013-11-28 08:49:49 PST
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 User image Dão Gottwald [:dao] 2013-11-28 08:51:56 PST
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 User image Mike de Boer [:mikedeboer] 2013-12-04 06:09:09 PST
> >+/* 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 User image Tim Nguyen :ntim 2013-12-04 09:08:59 PST
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 User image Justin Dolske [:Dolske] 2013-12-12 00:45:35 PST
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 User image Mike de Boer [:mikedeboer] 2013-12-12 02:10:31 PST
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 User image Mike de Boer [:mikedeboer] 2013-12-12 07:03:34 PST
Okay, this *would* be today if I could compile m-c on Windows 8.1. I already regret upgrading.
Comment 71 User image Stephen Horlander [:shorlander] 2013-12-12 07:10:04 PST
(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 User image Mike de Boer [:mikedeboer] 2013-12-13 09:39:20 PST
Created attachment 8347327 [details] [diff] [review]
Patch 1.2: Windows 8 toolkit theme adjustments
Comment 73 User image Mike de Boer [:mikedeboer] 2013-12-13 09:54:17 PST
(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 User image Mike de Boer [:mikedeboer] 2013-12-13 09:57:32 PST
(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 User image Jared Wein [:jaws] (please needinfo? me) 2013-12-13 10:41:06 PST
Comment on attachment 8347327 [details] [diff] [review]
Patch 1.2: Windows 8 toolkit theme adjustments

Over to a toolkit reviewer.
Comment 76 User image Tim Nguyen :ntim 2013-12-13 12:05:34 PST
(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 User image Dão Gottwald [:dao] 2013-12-14 03:36:33 PST
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 User image Dão Gottwald [:dao] 2013-12-14 04:12:55 PST
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 User image Tim Nguyen :ntim 2013-12-14 09:05:34 PST
(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 User image Tim Nguyen :ntim 2013-12-14 09:06:04 PST
(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 User image Mike de Boer [:mikedeboer] 2013-12-16 04:45:00 PST
Created attachment 8348006 [details] [diff] [review]
Patch 1.3: Windows 8 toolkit theme adjustments
Comment 82 User image Mike de Boer [:mikedeboer] 2013-12-17 07:21:45 PST
Created attachment 8348784 [details] [diff] [review]
Patch 2.6: Windows 8 browser theme adjustments
Comment 83 User image Mike de Boer [:mikedeboer] 2013-12-17 07:28:10 PST
Created attachment 8348788 [details] [diff] [review]
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 User image Dão Gottwald [:dao] 2013-12-18 05:06:36 PST
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 User image Peter Henkel [:Terepin] 2014-01-03 04:32:32 PST
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...).
Comment 86 User image :Gijs 2014-01-03 04:49:18 PST
(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 User image Peter Henkel [:Terepin] 2014-01-03 05:04:11 PST
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 User image Stanzilla 2014-01-03 09:33:20 PST
(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 User image Mike de Boer [:mikedeboer] 2014-01-06 03:06:55 PST
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 User image Guillaume C. [:ge3k0s] 2014-01-06 03:09:30 PST
Some parts aren't Win 8 specific though. AFAIK the panels are too be consistent on every platform for example.
Comment 91 User image Peter Henkel [:Terepin] 2014-01-06 03:27:15 PST
(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 User image Mike de Boer [:mikedeboer] 2014-01-06 03:47:51 PST
(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 User image Stanzilla 2014-01-06 15:38:24 PST
(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 User image Mike de Boer [:mikedeboer] 2014-01-09 06:50:18 PST
(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 User image Dão Gottwald [:dao] 2014-01-09 06:58:28 PST
(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 User image Mike de Boer [:mikedeboer] 2014-01-09 07:27:24 PST
Created attachment 8357780 [details] [diff] [review]
Patch 1.4: Windows 8 toolkit theme adjustments
Comment 97 User image Dão Gottwald [:dao] 2014-01-09 07:33:30 PST
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 User image Mike de Boer [:mikedeboer] 2014-01-09 07:38:52 PST
Created attachment 8357784 [details] [diff] [review]
Patch 1.5: Windows 8 toolkit theme adjustments
Comment 99 User image Mike de Boer [:mikedeboer] 2014-01-10 06:55:43 PST
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.
Comment 100 User image Tim Nguyen :ntim 2014-01-11 13:46:22 PST
(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 User image Tim Nguyen :ntim 2014-01-11 13:51:24 PST
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 User image Mike de Boer [:mikedeboer] 2014-01-11 13:59:39 PST
(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 User image Tim Nguyen :ntim 2014-01-13 12:34:54 PST
(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 User image Tim Nguyen :ntim 2014-01-13 12:38:48 PST
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 User image Mike de Boer [:mikedeboer] 2014-01-13 15:01:33 PST
(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 User image Masatoshi Kimura [:emk] 2014-01-13 17:00:54 PST
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 User image Dão Gottwald [:dao] 2014-01-14 08:20:01 PST
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 User image :Gijs 2014-01-15 15:13:17 PST
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 User image Mike de Boer [:mikedeboer] 2014-01-16 09:08:08 PST
remote: https://hg.mozilla.org/mozilla-central/rev/9c3ddd178154
Comment 110 User image Ryan VanderMeulen [:RyanVM] 2014-01-16 10:50:56 PST
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 User image Tim Nguyen :ntim 2014-01-18 13:17:26 PST
(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 User image Mike de Boer [:mikedeboer] 2014-01-20 03:40:12 PST
Re-landed without padding change: https://hg.mozilla.org/integration/fx-team/rev/ecb27777014d
Comment 113 User image Guillaume C. [:ge3k0s] 2014-01-20 03:45:45 PST
(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 User image Mike de Boer [:mikedeboer] 2014-01-20 03:54:53 PST
(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 User image Mike de Boer [:mikedeboer] 2014-01-20 06:33:35 PST
(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 User image Phil Ringnalda (:philor) 2014-01-20 18:44:47 PST
https://hg.mozilla.org/mozilla-central/rev/ecb27777014d
Comment 117 User image Jacek Caban 2014-02-06 08:45:38 PST
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 User image Carsten Book [:Tomcat] 2014-02-07 01:26:44 PST
https://hg.mozilla.org/mozilla-central/rev/fc823ce7f913
Comment 184 User image Bogdan Maris, QA [:bogdan_maris] 2014-03-07 06:06:52 PST
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 User image henryfhchan 2014-03-29 13:02:43 PDT
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 User image Tim Nguyen :ntim 2014-03-29 13:28:13 PDT
(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 User image henryfhchan 2014-03-29 13:37:46 PDT
Filed bug https://bugzilla.mozilla.org/show_bug.cgi?id=989660

Note You need to log in before you can comment on or make changes to this bug.