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

RESOLVED FIXED in Firefox 51

Status

()

Firefox
Theme
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Mehmet, Assigned: Gijs)

Tracking

(Blocks: 1 bug)

51 Branch
Firefox 51
Unspecified
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
Created attachment 8784577 [details]
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.
(Reporter)

Updated

2 years ago
Component: Untriaged → Theme
OS: Unspecified → Mac OS X
(Assignee)

Updated

2 years ago
Blocks: 1275655
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment hidden (mozreview-request)
(Assignee)

Comment 2

2 years ago
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...
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

Comment 3

2 years ago
mozreview-review
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.
Attachment #8784876 - Flags: review?(mstange) → review+
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)
Attachment #8784876 - Flags: review?(mconley) → review+
(Assignee)

Updated

2 years ago
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 5

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3a45b6ea11f
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 6

2 years ago
(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

2 years ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ad701f25715c
fix titlebar button locations across titlebar/lwtheme/devedition state, r=mconley,mstange

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ad701f25715c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
You need to log in before you can comment on or make changes to this bug.