1398395 - The hover effect of the caption buttons does not span the entire height of the tab strip in non-maximized windows with compact density

Status: NEW

(Core :: Widget: Win32, defect, P3)

Description

[Oriol Brufau [:Oriol]]

Attachment: screenshot.png

Set density to compact, make sure the window is not maximized, hover the close button. It does not span the entire height, there are 2 pixels below.

The screenshot compares maximized (good) and non-maximized (bad). Enlarged by a factor of 4.

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=27adfa4ab2d882afaa3d2c9403855973d78d8c00&tochange=37ba4f932f57fc60bf27554b5d36df3c721df75f

Comment 1

[Dão Gottwald [:dao]]

I think the problem here is that -moz-appearance: -moz-window-titlebar; implies a minimum height of 30px but compact tabs have a height of 29px.

Jim, can we reduce the minimum title bar height?

Alternatively, Stephen, should we set the set the compact tab height to 30px, or just live with this bug?

Comment 2

[Jim Mathies [:jimm]]

(In reply to Dão Gottwald [::dao] from comment #1)
> I think the problem here is that -moz-appearance: -moz-window-titlebar;
> implies a minimum height of 30px but compact tabs have a height of 29px.
> 
> Jim, can we reduce the minimum title bar height?
> 
> Alternatively, Stephen, should we set the set the compact tab height to
> 30px, or just live with this bug?

Looks like we ask Windows for the dims [1], then tweak those a bit based on our needs [2]. We could try removing a pixel here but it might not work out.


[1] http://searchfox.org/mozilla-central/rev/1ebd2eff44617df3b82eea7d2f3ca1b60cc591a0/widget/windows/nsNativeThemeWin.cpp#2399
[2] http://searchfox.org/mozilla-central/rev/1ebd2eff44617df3b82eea7d2f3ca1b60cc591a0/widget/windows/nsNativeThemeWin.cpp#1613

Comment 3

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [::dao] from comment #1)
> I think the problem here is that -moz-appearance: -moz-window-titlebar;
> implies a minimum height of 30px but compact tabs have a height of 29px.
> 
> Jim, can we reduce the minimum title bar height?
> 
> Alternatively, Stephen, should we set the set the compact tab height to
> 30px, or just live with this bug?

Compact tabs should be 28px tall not including any borders. If we can't make the window close button match that height we should just live with the bug instead of making the tabs taller.

Comment 4

[Stephen Horlander [:shorlander]]

After looking into this I think something else is going wrong here. In Compact mode there is a 2px space *above* the tab strip that shouldn't be there. Which might be what is causing this.

https://cl.ly/2d21320q3W19

Comment 5

[Edo Karabas]

Yep, I made a comment about this here: https://bugzilla.mozilla.org/show_bug.cgi?id=1404074#c10
This space is basically the root cause of bug 1404074. 
In maximized mode is everything OK. No bug 1398395 and no bug 1404074. 
The goal should be to move everything a bit "up" for the non-maximized mode, so the caption buttons will span entire height of the title bar and space above tabs will disappear.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Stephen Horlander [:shorlander] from comment #4)
> After looking into this I think something else is going wrong here. In
> Compact mode there is a 2px space *above* the tab strip that shouldn't be
> there. Which might be what is causing this.
> 
> https://cl.ly/2d21320q3W19

-moz-appearance: -moz-window-titlebar behind the tab strip is causing it, hence my questions in comment 1.

Comment 7

[Stephen Horlander [:shorlander]]

(In reply to Dão Gottwald [::dao] from comment #6)
> (In reply to Stephen Horlander [:shorlander] from comment #4)
> > After looking into this I think something else is going wrong here. In
> > Compact mode there is a 2px space *above* the tab strip that shouldn't be
> > there. Which might be what is causing this.
> > 
> > https://cl.ly/2d21320q3W19
> 
> -moz-appearance: -moz-window-titlebar behind the tab strip is causing it,
> hence my questions in comment 1.

I see, the bug title and the image wasn't clear, I thought it was a problem specific to the caption button height. Yeah we should fix this.