1111138 - [Windows 7/8] Window frame is slightly taller when using a lightweight theme

Status: RESOLVED WONTFIX

(Firefox :: Theme, defect)

Description

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: height-difference.png

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.

Comment 1

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

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.

Comment 2

[Blake Winton (:bwinton) (:☕️)]

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?  ;)

Comment 3

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

(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? :-)

Comment 4

[Blake Winton (:bwinton) (:☕️)]

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!  :)

Comment 6

[obongofon]

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

Comment 7

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

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.

Comment 8

[obongofon]

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

Comment 9

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

(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?

Comment 10

[obongofon]

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

Comment 11

[Ivan]

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

Comment 12

[Ivan]

Attachment: First patch probably not correct need feedback

Comment 13

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

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?

Comment 14

[Ivan]

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

Comment 15

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

Hey Ivan, do you need more help?

Comment 17

[Rishi Ahya]

Hi

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


thanks

Comment 18

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

(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.

Comment 19

[Jeff Griffiths (:canuckistani) (:⚡︎)]

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

Comment 20

[Aruna]

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

Comment 21

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

(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.

Comment 22

[Agboola Mukhtar <agbo-ola>]

Hi, I'm new to open-source and I'd like to work on this. I use windows 10

Comment 23

[Agboola Mukhtar <agbo-ola>]

Does the problem also occur on windows 10?

Comment 24

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

(In reply to Agboola Mukhtar from comment #23)

Does the problem also occur on windows 10?

No, this is specific to Windows 7/8

Comment 25

[vitorfront97]

Hi, can i be assigned to this bug?

Comment 26

[Gregory Pappas [:gregp]]

Resolving this bug as WONTFIX because Firefox versions >= 116 do not support Windows 7-8.1.