1297847 - Mac: window control buttons (caption / traffic light buttons) in private browsing windows are 1px too low when title bar is active/visible

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Mehmet]

Attachment: Bildschirmfoto 2016-08-24 um 22.20.19.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/601.7.7 (KHTML, like Gecko) Version/9.1.2 Safari/601.7.7

Steps to reproduce:

Mac OS 10.11.6
Firefox Nightly 51.0a1 (2016-08-24)


1.) Open a Normal Window
2.) Open an Incognito Window
3.) Activate "Show Title Bar" in the Customizing
4.) Compare the height of the Window Control Buttons in both windows



Actual results:

The Window Control Buttons (traffic-lights) in Incognito Windows are 1px too low.


Expected results:

They must be 1px higher, to match the height of the buttons of the Normal Window.

A screenshot is attached.

Comment 1

[:Gijs (he/him)]

Attachment: Bug 1297847 - fix titlebar button locations across titlebar/lwtheme/devedition state,

Review commit: https://reviewboard.mozilla.org/r/74184/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/74184/

Comment 2

[:Gijs (he/him)]

Markus, this is effectively your patch from bug 1275655 but with the -moz-box-align also put in the theme's browser.css.

AFAICT this gets us correct behaviour everywhere (lwtheme (devedition and non-devedition, which behave differently...), no lwtheme, private browser, no private browser, titlebar, no titlebar).

I think the solution to the mystery in bug 1275655 (the 1px offset I was seeing) is exactly this bug, and the reason we were seeing different things is because one of us had bug 1275650 applied as well, and one didn't...

Comment 3

[Markus Stange [:mstange]]

Comment on attachment 8784876 [details]
Bug 1297847 - fix titlebar button locations across titlebar/lwtheme/devedition state,

https://reviewboard.mozilla.org/r/74184/#review72096

(In reply to :Gijs Kruitbosch (away 26-29 incl.) from comment #2)
> Markus, this is effectively your patch from bug 1275655 but with the
> -moz-box-align also put in the theme's browser.css.

Oh wow, I had completely forgotten about that bug. I can't believe that it was only three months ago.

> AFAICT this gets us correct behaviour everywhere (lwtheme (devedition and
> non-devedition, which behave differently...), no lwtheme, private browser,
> no private browser, titlebar, no titlebar).

Nice, and it simplifies the code, too.

> I think the solution to the mystery in bug 1275655 (the 1px offset I was
> seeing) is exactly this bug, and the reason we were seeing different things
> is because one of us had bug 1275650 applied as well, and one didn't...

Oh! Yes, that may have been the reason.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8784876 [details]
Bug 1297847 - fix titlebar button locations across titlebar/lwtheme/devedition state,

https://reviewboard.mozilla.org/r/74184/#review72418

This looks good - though I suspect it'd be a good idea to run this through the mozscreenshot suite to make sure we didn't miss any cases (I tested default, lw-theme, private browsing, all with tabs in titlebar and without)

Comment 5

[:Gijs (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3a45b6ea11f

Comment 6

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #4)
> Comment on attachment 8784876 [details]
> Bug 1297847 - fix titlebar button locations across
> titlebar/lwtheme/devedition state,
> 
> https://reviewboard.mozilla.org/r/74184/#review72418
> 
> This looks good - though I suspect it'd be a good idea to run this through
> the mozscreenshot suite to make sure we didn't miss any cases (I tested
> default, lw-theme, private browsing, all with tabs in titlebar and without)

Seems like mozscreenshot doesn't test private browsing windows. I'll file a bug. The non-private ones all show up with no differences, so I think this is good to land.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad701f25715c
fix titlebar button locations across titlebar/lwtheme/devedition state, r=mconley,mstange

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ad701f25715c