We need to shift the painting of the persona background-image down one or two pixels, it currently covers the window border. We have moz extensions that may be able to help: https://developer.mozilla.org/en/css/background-image Otherwise we might need to create a new extension.

Here are 5 different implementations to adjust the window border and button position, based on the mockups here and the original bug 513170, and also an extra one as suggested to show the window glass resizer borders. Shorlander, can you take a look at these and tell what you think is the most correct one?

My personal take is a or e, leaning toward e until we have the rest of the frame skinned.

My vote is for A. Ciao!

I’m for A as well. Maybe the transparency might be a bit less then 50% but definitely not solid white as in C.

CC'ing faaborg for feedback on the screenshots

>CC'ing faaborg for feedback on the screenshots stephen should make the call on this

Comment on attachment 471221 [details] 5 different implementations A is correct. Actually 0.6 alpha is probably closer to what the system border does.

Implemented style A with 0.6 alpha on the inner border. Since the attempts of shifting the bg-color and -image wouldn't work in practice, the solution is to draw the top borders of the window ourselves. I already wrote and tested this one on top of patch from bug 593950

FTR, I updated the patch comment to say: - /* Redraw borders */ + /* Artificially draw window borders that are covered by lwtheme */

Comment on attachment 472845 [details] [diff] [review] Patch v1 >+ #main-window[chromemargin^="0,"] { >+ border-top: 1px solid transparent; >+ } >+ >+ #main-window[chromemargin^="0,"] > #titlebar > #titlebar-content { >+ border-top: 1px solid transparent; >+ } >+ >+ #main-window[chromemargin^="0,"] > #titlebar > #titlebar-content > #appmenu-button-container { >+ margin-top: -1px !important; >+ } >+ >+ #main-window[chromemargin^="0,"] > #titlebar { >+ margin-bottom: -1px; >+ } Why is all this applied if there's no lwtheme? >+ #main-window[sizemode="normal"]:-moz-lwtheme > #titlebar > #titlebar-content { >+ border-top-color: rgba(255, 255, 255, 0.6); >+ } move :-moz-lwtheme to #titlebar-content

Is bug 547805 considered in this work, or does it not affect the positioning of the background?

(In reply to comment #10) > Comment on attachment 472845 [details] [diff] [review] > Patch v1 > > >+ #main-window[chromemargin^="0,"] { > >+ border-top: 1px solid transparent; > >+ } > >+ > >+ #main-window[chromemargin^="0,"] > #titlebar > #titlebar-content { > >+ border-top: 1px solid transparent; > >+ } > >+ > >+ #main-window[chromemargin^="0,"] > #titlebar > #titlebar-content > #appmenu-button-container { > >+ margin-top: -1px !important; > >+ } > >+ > >+ #main-window[chromemargin^="0,"] > #titlebar { > >+ margin-bottom: -1px; > >+ } > > Why is all this applied if there's no lwtheme? Because the positioning of the elements is affected by the borders added, so I preferred to have a single final layout and make the borders transparent/not transparent, rather than having to shift things up/down when the lwtheme is applied. > > >+ #main-window[sizemode="normal"]:-moz-lwtheme > #titlebar > #titlebar-content { > >+ border-top-color: rgba(255, 255, 255, 0.6); > >+ } > > move :-moz-lwtheme to #titlebar-content Ok

Is the drawing error in the attachment related to this bug, or should I file a new bug?

Comment on attachment 472845 [details] [diff] [review] Patch v1 In order to prevent accidental height increases or alignment changes (now or in the future), please apply your code only in the :-moz-lwtheme case.

New patch, with a much better solution now. Before, to draw the two-colors border I was using the border of both the window and the titlebar-content. This lead to some tricky positioning issues, but now I'm using the -moz-border-top-colors to draw both of them to the titlebar-content, which simplifies things as it doesn't deal with the root element anymore. (In reply to comment #14) > In order to prevent accidental height increases or alignment changes (now or in > the future), please apply your code only in the :-moz-lwtheme case. I've applied everything to the :-moz-lwtheme case now. FWIW the original idea to apply the borders on both cases was exactly to prevent accidental changes (avoiding a different layout for lwtheme), but I'm fine with it either way. (In reply to comment #13) > Created attachment 476512 [details] > Drawing in the XP title bar > > Is the drawing error in the attachment related to this bug, or should I file a > new bug? I took a look and tried to do it together here, but turns out that'll need some trickier changes so it's probably better done in a separate bug.

(In reply to comment #15) > (In reply to comment #14) > > In order to prevent accidental height increases or alignment changes (now or in > > the future), please apply your code only in the :-moz-lwtheme case. > > I've applied everything to the :-moz-lwtheme case now. FWIW the original idea > to apply the borders on both cases was exactly to prevent accidental changes > (avoiding a different layout for lwtheme), but I'm fine with it either way. The point is that the toolbar height should remain the same as it is now, not just consistent between lwtheme and non-lwtheme.

Comment on attachment 479622 [details] [diff] [review] Patch v2 >--- a/browser/themes/winstripe/browser/browser-aero.css >+++ b/browser/themes/winstripe/browser/browser-aero.css > %define customToolbarColor hsl(214,44%,87%) >+%define aeroActiveBorderColor rgb(37, 44, 51) >+%define aeroInactiveBorderColor rgb(102, 102, 102) s/aero/glass/ >+ /* Artificially draw window borders that are covered by lwtheme, see bug 591930. */ >+ #main-window[chromemargin^="0,"][sizemode="normal"] > #titlebar > #titlebar-content:-moz-lwtheme { >+ border-top-width: 2px; >+ border-top-style: solid; border-top: 2px solid; >+ #main-window[chromemargin^="0,"][sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container:-moz-lwtheme { >+ margin-top: -1px; >+ } Won't this still push the toolbars down by one pixel?

(In reply to comment #17) > Comment on attachment 479622 [details] [diff] [review] > Patch v2 > > >--- a/browser/themes/winstripe/browser/browser-aero.css > >+++ b/browser/themes/winstripe/browser/browser-aero.css > > > %define customToolbarColor hsl(214,44%,87%) > >+%define aeroActiveBorderColor rgb(37, 44, 51) > >+%define aeroInactiveBorderColor rgb(102, 102, 102) > > s/aero/glass/ Actually, "aero" might be correct as well, assuming it's the same border color for basic and glass. We only use it for glass, though.

> > s/aero/glass/ > > Actually, "aero" might be correct as well, assuming it's the same border color > for basic and glass. We only use it for glass, though. They're different colors; I'll change to glass { > >+ border-top-width: 2px; > >+ border-top-style: solid; > > border-top: 2px solid; ok > > >+ #main-window[chromemargin^="0,"][sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container:-moz-lwtheme { > >+ margin-top: -1px; > >+ } > > Won't this still push the toolbars down by one pixel? No, the border-top isn't actually pushing anything, because the height of the #titlebar-content is 23 and #titlebar is 30, so any border <= 7 doesn't make a difference. I could add a margin-bottom: -2px (to #titlebar-content) to guarantee this in case the #titlebar shrinks in the future. This margin-top: -1px is just to reposition the appmenu button. It's -1px rather than -2px because it's natural position (without the borders) is 1px.

Follow-up needed due to landing of bug 601603, as discussed. I've realized though that we can remove the [chromemargin^="0,"] selector from these patches as the #titlebar element is not displayed at all when the menu bar is on. Do you want me to remove them from the patches?

(In reply to comment #20) > Do you want me to remove them from the patches? Yes please. Could you also merge the patches?

Sure, here is the merged patch with chromemargin selectors removed

Comment on attachment 485061 [details] [diff] [review] Merged >+ #main-window[sizemode="normal"] > #titlebar > #titlebar-content:-moz-lwtheme { >+ border-top: 2px solid; >+ -moz-border-top-colors: @glassActiveBorderColor@ rgba(255,255,255,.6); >+ margin-bottom: -2px; >+ } margin-bottom: -2px; doesn't seem to make a difference. r=me with this removed or explained.

I removed the margin-bottom: -2px. It was explained in comment 19, but it's not necessary so I kept that off. http://hg.mozilla.org/mozilla-central/rev/28d6bc4f438a

Huh, so that annoying white line is how it was designed??? Well, anyway, "border-top: 2px solid;" for Win7 causes bug 1199374