This prpbblem happens on Windows10TP build 10130. Steps To Reproduce 1. Open Sidebar (Ctrl+B) Actual Results: Glitch happens in Background color of toolbox and titlebar

In bug 1167929 for Thunderbird I found the -moz-appearance: -moz-win-exclude-glass; is steering the white area. It's applied now on #appcontent. If it would be applied on one layer above, #browser, also the sidebars would be included in the white area.

Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao

FWIW, I am not particularly happy with this patch, but it does address both this and bug 1167218. I don't *think* (but would love to be corrected) that it's possible to have a media query for "windows 10 and later" -- I guess this: @media not all and (-moz-os-version: windows-vista) { @media not all and (-moz-os-version: windows-win7) { @media not all and (-moz-os-version: windows-win8) { } } } inside the compositor one would work, but it seemed so ugly that the solution in the patch seemed nicer. Let me know if you disagree.

Can you explain the use of -moz-win-glass and -moz-win-exclude-glass on Windows 10? Preferably in a CSS comment, not just here.

(In reply to Dão Gottwald [:dao] from comment #4) > Can you explain the use of -moz-win-glass and -moz-win-exclude-glass on > Windows 10? Preferably in a CSS comment, not just here. So when I went and looked at this, I saw no reason to not use the win10 styling on win8 as well. Then I went to look at win7. I found bug 633282 comment 146, but actually, when I test with current trunk and adding -moz-win-exclude-glass to #browser instead of #appcontent in the browser devtools, I don't see the side borders becoming opaque - they seem to remain transparent. Which, as best I can tell, means we can dispense with treating anything specially for -moz-win-exclude-glass, and only specialcase vista+win7 for borderless-glass instead of with-border-glass. Is that right, Dão, or am I missing something?

(In reply to :Gijs Kruitbosch from comment #5) > Then I went to look at win7. I found bug 633282 comment 146, but actually, > when I test with current trunk and adding -moz-win-exclude-glass to #browser > instead of #appcontent in the browser devtools, I don't see the side borders > becoming opaque - they seem to remain transparent. This doesn't make sense to me. Like I said in that comment, #browser-border-start and #browser-border-end are children of #browser, so if -moz-win-exclude-glass works like it should, it would make the area behind that borders transparent.

(In reply to Dão Gottwald [:dao] from comment #6) > (In reply to :Gijs Kruitbosch from comment #5) > > Then I went to look at win7. I found bug 633282 comment 146, but actually, > > when I test with current trunk and adding -moz-win-exclude-glass to #browser > > instead of #appcontent in the browser devtools, I don't see the side borders > > becoming opaque - they seem to remain transparent. > > This doesn't make sense to me. No, me neither. But that's what I'm seeing. It sounds like you prefer I leave the patch as-is in that respect? > Like I said in that comment, > #browser-border-start and #browser-border-end are children of #browser, so > if -moz-win-exclude-glass works like it should, it would make the area > behind that borders transparent. ITYM opaque?

(In reply to :Gijs Kruitbosch from comment #7) > (In reply to Dão Gottwald [:dao] from comment #6) > > (In reply to :Gijs Kruitbosch from comment #5) > > > Then I went to look at win7. I found bug 633282 comment 146, but actually, > > > when I test with current trunk and adding -moz-win-exclude-glass to #browser > > > instead of #appcontent in the browser devtools, I don't see the side borders > > > becoming opaque - they seem to remain transparent. > > > > This doesn't make sense to me. > > No, me neither. But that's what I'm seeing. It sounds like you prefer I > leave the patch as-is in that respect? Yes, -moz-win-exclude-glass shouldn't be set on #browser. > > Like I said in that comment, > > #browser-border-start and #browser-border-end are children of #browser, so > > if -moz-win-exclude-glass works like it should, it would make the area > > behind that borders transparent. > > ITYM opaque? Yes, opaque

Comment on attachment 8614932 [details] MozReview Request: Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao

(Now with comments.)

What does "We use glass on win8+" even mean? There's no glass on Windows 8...

Comment on attachment 8614932 [details] MozReview Request: Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao

(In reply to Dão Gottwald [:dao] from comment #11) > What does "We use glass on win8+" even mean? There's no glass on Windows 8... Updated to be more explicit. If this is still not right, can you make a concrete suggestion about what you think the text should be saying? Thanks!

You're assuming I know your patch is correct, but I don't. ;) I was hoping you knew why you used -moz-win-glass. I don't think "in order for Windows to draw the native caption buttons" is correct, as Windows should draw them regardless. "Theme colors for the nonclient area (titlebar)" doesn't sound quite right either, since the title bar is actually client area for us, e.g. we render tabs up there. Also, is -moz-win-exclude-glass needed at all on Windows 8 and 10?

(In reply to Dão Gottwald [:dao] from comment #14) > You're assuming I know your patch is correct, but I don't. ;) I was hoping > you knew why you used -moz-win-glass. I don't think "in order for Windows to > draw the native caption buttons" is correct, as Windows should draw them > regardless. If I remember correctly, the difference might be that -moz-win-glass tells our widget code to cut out a piece from our client area where Windows draws caption buttons. I'm not sure though.

(In reply to Dão Gottwald [:dao] from comment #14) > You're assuming I know your patch is correct, but I don't. ;) I was hoping > you knew why you used -moz-win-glass. I don't think "in order for Windows to > draw the native caption buttons" is correct, as Windows should draw them > regardless. So, this part I do actually know, and this is why this patch took me so long to write. I will try and write things down. I don't think it makes sense to copy all this into a CSS comment. If you don't use either -moz-win-glass or -moz-win-borderless-glass, the titlebar buttons do disappear. Here's why. We cut out a space for them in widget code iff we're using compositor. That code is here: http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#3537 Unfortunately, we tell windows to do something with it only if we're using glass/borderless-glass: http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#2690 . Note how we do not set the margins object to be nonempty or the "policy" variable to anything non-default if the window isn't using glass/borderless-glass. In fact, that function gets called from http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp#2628 which bails if !HasGlass, which again, checks moz-appearance for glass/borderless-glass ( http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.h#331) The code in the 2nd link also, as best I can tell, gives a clue as to why we're seeing this sidebar problem (and the thicker borders when using borderless-glass even when the sidebar is closed): the margins for glass are set to include a "kGlassMarginAdjustment", which is defined to 2. After talking this over with jimm, I tried using the XUL titlebar buttons, but asking the windows theme library for the glyphs for those returns "aero basic" style buttons even on win8/win10 (yeah, we were surprised, too...). > "Theme colors for the nonclient area (titlebar)" doesn't sound > quite right either, since the title bar is actually client area for us, e.g. > we render tabs up there. > > Also, is -moz-win-exclude-glass needed at all on Windows 8 and 10? At least on win10, without it the titlebar becomes the "accent" color (blue on my machine; it's probably whatever it would be on win8, except I expect the configuration options for it are gone, though I've not looked for them - all the titlebars of classic desktop apps are white; modern apps are using grey). In other words, it's making this bug happen but not just to the sidebar "part" of the titlebar, but to the entire thing. An alternative would be messing about in widget to try and convince it to do the right thing in terms of drawing our titlebars even when not using -moz-appearance: -moz-win-glass, but this seemed the simpler solution, especially for uplift. That said... I foresee a future where we will need to make the XUL options work. The titlebar button cutout looks ugly with lightweight themes because the background is either white or the glass accent color (neither of which jives well with it), and has this silly rounded corner at the bottom left (the source of which I have not yet been able to find, though I've not focused too much on looking for it). On top of that, the design that UX has made has us using the "Modern-style" dark grey-ish titlebar, which will take further work as well. All that said... I still think this patch is worth taking because it fixes these two very visible bugs at very little cost in complexity, and is upliftable to 39. We can investigate the more complicated fixes, but that will take more time and testing still. Does all this make sense and/or help?

Dão, can you clarify how you would like to move forward here? I would ideally like to fix these issues for 39 still, as they are pretty glaring, and the CSS patch is low-risk.

(In reply to :Gijs Kruitbosch from comment #16) > (In reply to Dão Gottwald [:dao] from comment #14) > > You're assuming I know your patch is correct, but I don't. ;) I was hoping > > you knew why you used -moz-win-glass. I don't think "in order for Windows to > > draw the native caption buttons" is correct, as Windows should draw them > > regardless. > > So, this part I do actually know, and this is why this patch took me so long > to write. I will try and write things down. I don't think it makes sense to > copy all this into a CSS comment. > > If you don't use either -moz-win-glass or -moz-win-borderless-glass, the > titlebar buttons do disappear. Here's why. > > We cut out a space for them in widget code iff we're using compositor. That > code is here: > http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow. > cpp#3537 > > Unfortunately, we tell windows to do something with it only if we're using > glass/borderless-glass: > http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow. > cpp#2690 . Note how we do not set the margins object to be nonempty or the > "policy" variable to anything non-default if the window isn't using > glass/borderless-glass. > > In fact, that function gets called from > http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow. > cpp#2628 which bails if !HasGlass, which again, checks moz-appearance for > glass/borderless-glass ( > http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.h#331) Part of that story still sounds backwards or at least misleading. It's my understanding that the chromemargin attribute effectively gets rid of the title bar, so there's no non-client area as far as we're concerned (although widget code might still somehow make this distinction for reasons that shouldn't matter to us). And then, I believe we need to use -moz-win-(borderless-)glass such that we get transparency rather than a standard opaque window background. I'm not sure where exactly in nsWindow::UpdateGlass we tell "Windows to draw the native caption buttons".

(In reply to Dão Gottwald [:dao] from comment #18) > (In reply to :Gijs Kruitbosch from comment #16) > Part of that story still sounds backwards or at least misleading. It's my > understanding that the chromemargin attribute effectively gets rid of the > title bar, so there's no non-client area as far as we're concerned (although > widget code might still somehow make this distinction for reasons that > shouldn't matter to us). And then, I believe we need to use > -moz-win-(borderless-)glass such that we get transparency rather than a > standard opaque window background. I'm not sure where exactly in > nsWindow::UpdateGlass we tell "Windows to draw the native caption buttons". Well, without the patch we use borderless-glass. The patch switches to non-borderless glass for win8/win10. I do not follow why our understanding of *how* widget deals with caption button drawing is relevant here, beyond "if you don't specify either glass or borderless glass, the caption buttons disappear on compositor-only operating systems like win8/win10". That is just a fact. I tested it, it breaks. The patch fixes the issue at hand (and another one besides), and so in that sense it is certainly "correct". Can we take it or do you think it is absolutely necessary to do a deep-dive into the ways and means of widget in order to move forward here, and if so, why? I'm happy to do this kind of investigation, I would just seriously prefer to land this fix first (as it is a strict improvement on the status quo) and do more digging in a separate bug (like, say, the one where we try to use different theming on windows 10 and will run into similar issues again).

I'm mostly fine with the code changes at this point (but see below). I just don't want the documentation to be misleading since that's arguably worse than having no documentation. So, how about you just drop the claim about us telling Windows to render caption buttons? I also think you can remove the part about -moz-win-borderless-glass since the difference between that and -moz-win-glass is self-explanatory as far as Windows Vista and 7 are concerned. (In reply to :Gijs Kruitbosch from comment #16) > > Also, is -moz-win-exclude-glass needed at all on Windows 8 and 10? > > At least on win10, without it the titlebar becomes the "accent" color Does not using -moz-win-exclude-glass really do that or are you talking about -moz-windows-glass?

(In reply to Dão Gottwald [:dao] from comment #20) > I'm mostly fine with the code changes at this point (but see below). I just > don't want the documentation to be misleading since that's arguably worse > than having no documentation. So, how about you just drop the claim about us > telling Windows to render caption buttons? I also think you can remove the > part about -moz-win-borderless-glass since the difference between that and > -moz-win-glass is self-explanatory as far as Windows Vista and 7 are > concerned. > > (In reply to :Gijs Kruitbosch from comment #16) > > > Also, is -moz-win-exclude-glass needed at all on Windows 8 and 10? > > > > At least on win10, without it the titlebar becomes the "accent" color > > Does not using -moz-win-exclude-glass really do that or are you talking > about -moz-windows-glass? I find this confusing as well. I'm seeing that basically, the width of the portion of the titlebar that is blue/red/... or whatever the accent color is corresponds to the width of the portion of the window that does *not* have exclude-glass. So pre-patch, the sidebar isn't part of the exclude-glass region, and so when the sidebar is opened the part of the window that is "above" the sidebar has the accent color (see screenshot in this bug). The width of that area corresponds to the sidebar width (the part that doesn't have exclude-glass). If I remove exclude-glass entirely, the entire width of the titlebar is painted in this accent color. This happens irrespective of whether I use -moz-win-borderless-glass or -moz-win-glass as the appearance value for the window (note that -moz-windows-glass (with "dows") is a media-query-able thing that is not involved here, as best I can tell, and is different from the -moz-appearance value -moz-win-glass). Does this clarify?

(In reply to :Gijs Kruitbosch from comment #21) > > > > Also, is -moz-win-exclude-glass needed at all on Windows 8 and 10? > > > > > > At least on win10, without it the titlebar becomes the "accent" color > > > > Does not using -moz-win-exclude-glass really do that or are you talking > > about -moz-windows-glass? > > I find this confusing as well. I'm seeing that basically, the width of the > portion of the titlebar that is blue/red/... or whatever the accent color is > corresponds to the width of the portion of the window that does *not* have > exclude-glass. So pre-patch, the sidebar isn't part of the exclude-glass > region, and so when the sidebar is opened the part of the window that is > "above" the sidebar has the accent color (see screenshot in this bug). The > width of that area corresponds to the sidebar width (the part that doesn't > have exclude-glass). > > If I remove exclude-glass entirely, the entire width of the titlebar is > painted in this accent color. This happens irrespective of whether I use > -moz-win-borderless-glass or -moz-win-glass as the appearance value for the > window (note that -moz-windows-glass (with "dows") is a media-query-able > thing that is not involved here, as best I can tell, and is different from > the -moz-appearance value -moz-win-glass). > > Does this clarify? yes, thanks

Comment on attachment 8614932 [details] MozReview Request: Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao

I've removed comments re glass/borderless-glass. Let me know if this was too drastic, and if you're looking for something else instead.

Gijs do you want to request uplift for this? Let me know if you think it is good to go. If not for beta then maybe for aurora. Thanks!

Comment on attachment 8614932 [details] MozReview Request: Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao Approval Request Comment [Feature/regressing bug #]: windows 10 [User impact if declined]: confusing coloring in the titlebar, esp. when the sidebar is open, when using Firefox on win10 [Describe test coverage new/current, TreeHerder]: nope, style change only [Risks and why]: low, CSS-only change for win8 and win10. One known problem exposed by this bug is a graphics issue on win8/win10 VMs when first opening a sidebar in fullscreen mode only (bug 1173339). Considering this seems to be restricted to a much smaller set of setups, is transient and only occurs in a subset of the window sizes this problem occurs in, the tradeoff seems clear. [String/UUID change made/needed]: no.

Comment on attachment 8614932 [details] MozReview Request: Bug 1169911 - fix windows 10 titlebar coloring/border issues, r?dao Approved for uplift to aurora and beta. css fix.

Reproduced this on latest Aurora 40.0a2, build ID: 20150614004003 Confirming the fix on latest Nightly, build ID: 20150614030204.

Liz, can you move the tracking flags from bug 1167218 to here, please?

Alo verified on latest Aurora, build ID: 20150621004005 and on Firefox 39 beta 7, builD ID: 20150618135210.