Closed
Bug 1402309
Opened 3 years ago
Closed 3 years ago
On macOS 10.9 (mavericks), fullscreen button is not vertically centered nor anchored to the top while it should, as it was
Categories
(Firefox :: Theme, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 58
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | - | verified |
firefox58 | --- | verified |
People
(Reporter: clement.lefevre, Assigned: yzen)
References
(Blocks 1 open bug)
Details
(Keywords: nightly-community, Whiteboard: [mozfr-community] [reserve-photon-visual])
Attachments
(8 files)
[Tracking Requested - why for this release]: After bug 1396240 got fixed and sign went back, I could see that the fullscreen button appear to be a lot lower than it should (it's actually lower than in normal windows), hence not vertically centered on the bar. See the joined screenshot to have an overview. In the meantime, private browsing sign and fullscreen button now looks very very close, is this normal? Same question for the private browing sign that takes the full height on compact density while it wasn't before if I remember correctly, is this intended? Screenshot was taken on OS X 10.9.
Reporter | ||
Updated•3 years ago
|
OS: Unspecified → Mac OS X
Updated•3 years ago
|
Whiteboard: [mozfr-community][photon-visual] → [mozfr-community] [photon-visual] [triage]
Comment 1•3 years ago
|
||
(In reply to Clément Lefèvre from comment #0) > Created attachment 8911152 [details] > Private browing sign and fullscreen button on OS X 10.9 It looks like this screenshot isn't from the latest nightly build.
Reporter | ||
Comment 2•3 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #1) > (In reply to Clément Lefèvre from comment #0) > > Created attachment 8911152 [details] > > Private browing sign and fullscreen button on OS X 10.9 > > It looks like this screenshot isn't from the latest nightly build. No it isn't, I reused the one I posted on https://bugzilla.mozilla.org/show_bug.cgi?id=1396240#c7 It's then nine days old. It was still the same at the time I posted that bug bug it's right I didn't restarted my Nightly since one day or two for whatever reason. That's now done and I attached a new screenshot. Private browsing badge indeed changed to a circular version so I guess that part isn't important anymore. About the fullscreen button, at the time of that earlier comment it was at a different position between normal window and private browsing window. It is now at the same position, but at the very bottom of the bar. In my humble opinion it should be either vertically centered or anchored to the top of the bar rather than the bottom, closer to where it was before. I'll edit the title to adjust with those changes.
Reporter | ||
Updated•3 years ago
|
Summary: On macOS, fullscreen button on private browsing windows is not vertically centered and the badge take the full height → On macOS, fullscreen button is not vertically centered nor anchored to the top while it should, as it was
Updated•3 years ago
|
Flags: qe-verify+
Priority: -- → P3
QA Contact: ovidiu.boca
Whiteboard: [mozfr-community] [photon-visual] [triage] → [mozfr-community] [reserve-photon-visual]
Updated•3 years ago
|
Blocks: photon-visual
Priority: P3 → P4
Comment 3•3 years ago
|
||
P4, not tracking.
status-firefox55:
--- → unaffected
status-firefox56:
--- → unaffected
status-firefox57:
--- → fix-optional
status-firefox58:
--- → fix-optional
status-firefox-esr52:
--- → unaffected
Assignee | ||
Comment 4•3 years ago
|
||
Attachment #8912329 -
Flags: review?(dao+bmo)
Assignee | ||
Updated•3 years ago
|
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
Comment 5•3 years ago
|
||
Comment on attachment 8912329 [details] [diff] [review] 1402309 patch Nice code simplification, thanks! Looks good to me if it works. I'm redirecting the review request to Johann since I can't currently test on Mac.
Attachment #8912329 -
Flags: review?(dao+bmo) → review?(jhofmann)
Updated•3 years ago
|
Priority: P4 → P1
Assignee | ||
Updated•3 years ago
|
Summary: On macOS, fullscreen button is not vertically centered nor anchored to the top while it should, as it was → On macOS 10.9 (mavericks), fullscreen button is not vertically centered nor anchored to the top while it should, as it was
Reporter | ||
Comment 6•3 years ago
|
||
:yzen, looking at the change of summary, this was not affecting other macOS versions?
Comment 7•3 years ago
|
||
Comment on attachment 8912329 [details] [diff] [review] 1402309 patch Review of attachment 8912329 [details] [diff] [review]: ----------------------------------------------------------------- I only tested on 10.12, but that works fine, thank you. The window controls have a very very slightly different vertical position than before, but that's ok and probably to be expected.
Attachment #8912329 -
Flags: review?(jhofmann) → review+
Assignee | ||
Comment 8•3 years ago
|
||
(In reply to Clément Lefèvre from comment #6) > :yzen, looking at the change of summary, this was not affecting other macOS > versions? This is the only version I could reproduce it with. Newer OSX builds (like the current 10.12) do not have a full screen button (unless it's added through customization, and it is not the same one).
Assignee | ||
Comment 9•3 years ago
|
||
Would you be able to try this version of Firefox and see if the issue is fixed in your environment? Thanks!
Flags: needinfo?(clement.lefevre)
Reporter | ||
Comment 10•3 years ago
|
||
(In reply to Yura Zenevich [:yzen] from comment #9) > Would you be able to try this version of Firefox and see if the issue is > fixed in your environment? Thanks! Which version are you referring to?
Flags: needinfo?(clement.lefevre)
Assignee | ||
Comment 11•3 years ago
|
||
OS X 10.9, since I could not display full screen button on newer osx.
Assignee | ||
Comment 12•3 years ago
|
||
urgh sorry I realized I did not include the image: https://queue.taskcluster.net/v1/task/PkyJqFtrTja1Omw2EmAS2g/runs/0/artifacts/public/build/target.dmg
Reporter | ||
Comment 14•3 years ago
|
||
(In reply to Yura Zenevich [:yzen] from comment #13) > See comment 12 No worries, I was just checking everything and trying to find older screenshots to compare with :) I can't find old enough ones to compare with. It might have been a little bit higher, but I'm not even sure and it looks pretty natural on your test build to be honest. As :johannh told, it moved window controls from one or two pixels vertically higher: I don't know if this is a problem, but it doesn't look particularly shocking (I guess it should be vertically centered but I don't know how to "measure" it). This is nitpick, but I'll let you decide if that's worth checking it.
Flags: needinfo?(clement.lefevre)
Comment 15•3 years ago
|
||
Pushed by yura.zenevich@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/87438bd3387b fix titlebar-fullscreen-button vertical alignment. r=johannh
Comment 16•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/87438bd3387b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment 17•3 years ago
|
||
Comment on attachment 8912329 [details] [diff] [review] 1402309 patch Approval Request Comment [Feature/Bug causing the regression]: photon tab strip and title bar changes [User impact if declined]: fullscreen button is misaligned on OS X 10.9 [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: [List of other uplifts needed for the feature/fix]: / [Is the change risky?]: hardly [Why is the change risky/not risky?]: pretty straightforward CSS fix (actually a code simplification) [String changes made/needed]: /
Attachment #8912329 -
Flags: approval-mozilla-beta?
Comment 18•3 years ago
|
||
Screenshots with the slightly moving window controls: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=64c8107a2a13895e9e4295806bf65b99639cb651&newProject=mozilla-central&newRev=57f68296c350469d73d788eb3695a898947b4acb
Comment 19•3 years ago
|
||
Comment on attachment 8912329 [details] [diff] [review] 1402309 patch Photon polish, taking it. Should be in 57b5
Attachment #8912329 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 20•3 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/2ad2dfbbb93a
Comment 21•3 years ago
|
||
I tested this on Mac OS X 10.9 with FF Nightly 58.0a1(2017-10-02) and here are my results: I measured the distance of the buttons(the 3 buttons, close, maximize and minimize) and I have 8px from the button and 10px from the top, please see the attachment. Johann this is the expected output? I also so that the buttons have a kind of a border in the bottom part, I mark it with a red arrow. Thanks
Flags: needinfo?(jhofmann)
Comment 22•3 years ago
|
||
Here is the attachment.
Comment 23•3 years ago
|
||
(In reply to ovidiu boca[:Ovidiu] from comment #21) > I tested this on Mac OS X 10.9 with FF Nightly 58.0a1(2017-10-02) and here > are my results: I measured the distance of the buttons(the 3 buttons, close, > maximize and minimize) and I have 8px from the button and 10px from the top, > please see the attachment. Johann this is the expected output? I also so > that the buttons have a kind of a border in the bottom part, I mark it with > a red arrow. > Thanks I noted the slight height difference in comment 7 but it's OK in my book. I'll redirect the window buttons question to Clément, since I don't know what it was like before (I'm not running 10.9 and we don't have screenshot tests there). I was under the impression that this is just the 10.9 style, is that correct?
Flags: needinfo?(jhofmann) → needinfo?(clement.lefevre)
Reporter | ||
Comment 24•3 years ago
|
||
(In reply to Johann Hofmann [:johannh] from comment #23) > (In reply to ovidiu boca[:Ovidiu] from comment #21) > > I tested this on Mac OS X 10.9 with FF Nightly 58.0a1(2017-10-02) and here > > are my results: I measured the distance of the buttons(the 3 buttons, close, > > maximize and minimize) and I have 8px from the button and 10px from the top, > > please see the attachment. Johann this is the expected output? I also so > > that the buttons have a kind of a border in the bottom part, I mark it with > > a red arrow. > > Thanks > > I noted the slight height difference in comment 7 but it's OK in my book. > I'll redirect the window buttons question to Clément, since I don't know > what it was like before (I'm not running 10.9 and we don't have screenshot > tests there). I was under the impression that this is just the 10.9 style, > is that correct? Well, honnestly I was not shocked by that at all, and I guess that's way more visible due to the black bar of the default/dark theme. I'm not very good at distinguishing this kind of details, so I'll just add screenshots of how it looks on Firefox with default and light theme, and then with two other applications to compare and let you decide whether it's considered normal and should be like that or not.
Flags: needinfo?(clement.lefevre)
Reporter | ||
Comment 25•3 years ago
|
||
Reporter | ||
Comment 26•3 years ago
|
||
Reporter | ||
Comment 27•3 years ago
|
||
Reporter | ||
Comment 28•3 years ago
|
||
Reporter | ||
Comment 29•3 years ago
|
||
Looking at above screenshots, I'll let Johann decide whether this is the normal/wished style or not.
Flags: needinfo?(jhofmann)
Comment 30•3 years ago
|
||
I think we're good. It looks like this is just a little artifact from the dark background and it doesn't seem to annoy people too much.
Flags: needinfo?(jhofmann)
Comment 31•3 years ago
|
||
Based on comment 23 and comment 30 I will mark this as verified fixed. Testing was made on Nightly 58.0a1(2017-10-07) with Mac OS X 10.9.
Comment 32•3 years ago
|
||
This is also verified on FF beta 57.0b9 with Mac OS X 10.9.
You need to log in
before you can comment on or make changes to this bug.
Description
•