(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo Reporter
	Description • 10 years ago

When using a lightweight theme on Windows 8 (possibly also other windows versions), the window frame grows by a few pixels (see attachment for comparison). This is particularly jarring when switching themes in customization mode.

The size of the window frame should be the same with lightweight themes as with the default theme.

	:Gijs (he/him)
	Comment 1 • 10 years ago

I bet this is caused by the lwt border that we add for tabsintitlebar and lwthemes because the window's top border doesn't get rendered. Conveniently, that's exactly 2px.

This will be aero+ specific.

	Blake Winton (:bwinton) (:☕️)
	Comment 2 • 9 years ago

So the code that needs changing is https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser-aero.css#297 or would we need to modify something like https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser.css#143 as well?

(Also, can I sign you up as a mentor for this bug?  ;)

	:Gijs (he/him)
	Comment 3 • 9 years ago

(In reply to Blake Winton (:bwinton) from comment #2)
> So the code that needs changing is
> https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/
> browser-aero.css#297 or would we need to modify something like
> https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/
> browser.css#143 as well?
> 
> (Also, can I sign you up as a mentor for this bug?  ;)

Are you sure? Because that's just a background image, so it doesn't really make sense...

I expect this is really:

http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser-aero.css#125

in which case it should reproduce in aero glass but not aero basic (on win7 - win8 will be affected everywhere). Can you verify this before we mark it mentored by me, so we don't send someone astray before fixing this? :-)

	Blake Winton (:bwinton) (:☕️)
	Comment 4 • 9 years ago

Yep, happens in aero glass: https://dl.dropboxusercontent.com/u/2301433/Screenshots/Bug1111138-Glass.png
Doesn't happen in aero classic: https://dl.dropboxusercontent.com/u/2301433/Screenshots/Bug1111138-Classic.png

Thanks!  :)

	obongofon
	Comment 6 • 9 years ago

Hello, can you assign me this bug as it will be interesting to work with

	Jared Wein [:jaws] (please needinfo? me)
	Comment 7 • 9 years ago

Hi obongofon, we usually wait until a patch is uploaded before assigning bugs to new contributors. Please go ahead and begin working on the patch and let us know if you have any questions.

	obongofon
	Comment 8 • 9 years ago

Please can you let me know which patch I should work on.
Thank you

	:Gijs (he/him)
	Comment 9 • 9 years ago

(In reply to obongofon from comment #8)
> Please can you let me know which patch I should work on.
> Thank you

You'll need to replace this code:

https://dxr.mozilla.org/mozilla-central/rev/35916735b8afc5b0732e00f9aeb56bf846bba7f4/browser/themes/windows/browser-aero.css#298-302

and this code:

https://dxr.mozilla.org/mozilla-central/rev/35916735b8afc5b0732e00f9aeb56bf846bba7f4/browser/themes/windows/browser-aero.css#393-402

with code that draws borders as a background-image (instead of a "normal" border) using a gradient. You can look at this code:

https://dxr.mozilla.org/mozilla-central/rev/35916735b8afc5b0732e00f9aeb56bf846bba7f4/browser/themes/windows/browser-aero.css#486-496

for inspiration.

Does that sound doable?

	obongofon
	Comment 10 • 9 years ago

yes. I will work on it and get back to you.
Thank you.

	Ivan
	Comment 11 • 9 years ago

I will try to create a patch for this bug and submit.

	:Gijs (he/him)
	Comment 13 • 9 years ago

Comment on attachment 8696837 [details] [diff] [review]
First patch probably not correct need feedback

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

So as noted in comment #9 we'll need to remove the "border" and "border-top" styles in all 3 places you identified, and use background-image instead. So in the top hunk:

::: browser/themes/windows/browser-aero.css
@@ +298,5 @@
>      /* Artificially draw window borders that are covered by lwtheme, see bug 591930.
>       * Borders for vista/win7 are below, win10 doesn't need them. */
>      #main-window[sizemode="normal"] > #tab-view-deck > #browser-panel:-moz-lwtheme {
> +      border-top: solid ( to top, @toolbarShadowColor@ 0, @toolbarShadowColor@ 1px, 
> +	  rgba (255,255,255,.6) 1px, rgba(255,255,255,.6) 2px, transparent 2px);

instead of starting with "border-top: solid)", start with:

background-image: linear-gradient


And then please test the changes. Do you have a windows 8 and/or windows 7 machine on which you can test enabling/disabling themes?

	Ivan
	Comment 14 • 9 years ago

I have windows 8.1 thank you for the feedback  i will try to submit a new patch by tomorrow.

	:Gijs (he/him)
	Comment 15 • 9 years ago

Hey Ivan, do you need more help?

	Rishi Ahya
	Comment 17 • 8 years ago

Hi

can you assign me to this bug, i would like to work on it?


thanks

	:Gijs (he/him)
	Comment 18 • 8 years ago

(In reply to Rishi Ahya from comment #17)
> Hi
> 
> can you assign me to this bug, i would like to work on it?
> 
> 
> thanks

Hi Rishi, let's start with bug 1317190.

	Jeff Griffiths (:canuckistani) (:⚡︎)
	Comment 19 • 7 years ago

This shouldn't block compact themes, it pre-date them and applies generally to lwt.

	Aruna
	Comment 20 • 7 years ago

Can I work on this bug? However I have the source code on linux. Will it be still reproducible?

	:Gijs (he/him)
	Comment 21 • 7 years ago

(In reply to Aruna from comment #20)
> Can I work on this bug? However I have the source code on linux. Will it be
> still reproducible?

No, this only happens on Windows 7 and 8.