1399567 - [macOS] [Photon] The Security text and the URL text are not centered vertically in the Location Bar

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Mehmet]

Attachment: actual_vs_expected.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3213.3 Safari/537.36

Steps to reproduce:

57.0a1 (2017-09-13) (64-Bit)
macOS 10.12.6

1.) Visit any page e.g. https://www.mozilla.org/
2.) Take a look at the position of the Security text (Mozilla Corporation [US]) and the URL (https://www.mozilla.org/de/)


Actual results:

They are not vertically aligned in the Location Bar.




Expected results:

Both texts could be 1px higher and if you highlight the URL, the highlight could have 1px more blue highlight at the bottom.

Screenshots are attached.

Comment 1

[Dão Gottwald [:dao]]

There's this old code adding more padding on the top than on the bottom on Mac:
http://searchfox.org/mozilla-central/rev/f6dc0e40b51a37c34e1683865395e72e7fca592c/browser/themes/osx/browser.css#372

We probably just need to remove this.

Comment 2

[Dão Gottwald [:dao]]

Attachment: Bug 1399567 - Remove uneven vertical padding from urlbar and searchbar input boxes on Mac.

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

Comment 3

[Johann Hofmann [:johannh]]

Comment on attachment 8912213 [details]
Bug 1399567 - Remove uneven vertical padding from urlbar and searchbar input boxes on Mac.

https://reviewboard.mozilla.org/r/183586/#review188766

Looks good to me.

Comment 4

[Mehmet]

Hello. Thank you for looking into it. Just only one question: I didn‘t mentioned that the text is also not vertically aligned in the Search Bar. Does your fix also consider the Search Bar or should I file a separate report for it? Thanks in advance.

Comment 5

[Dão Gottwald [:dao]]

(In reply to Mehmet from comment #4)
> Hello. Thank you for looking into it. Just only one question: I didn‘t
> mentioned that the text is also not vertically aligned in the Search Bar.
> Does your fix also consider the Search Bar

It doesn't but it should. Somehow I missed the relevant searchbar-specific code when I looked for it.

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8912213 [details]
Bug 1399567 - Remove uneven vertical padding from urlbar and searchbar input boxes on Mac.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183586/diff/1-2/

Comment 7

[Johann Hofmann [:johannh]]

Comment on attachment 8912213 [details]
Bug 1399567 - Remove uneven vertical padding from urlbar and searchbar input boxes on Mac.

https://reviewboard.mozilla.org/r/183586/#review189162

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fba4fab273da
Remove uneven vertical padding from urlbar and searchbar input boxes on Mac. r=johannh

Comment 9

[Phil Ringnalda (:philor)]

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

Comment 10

[Mehmet]

Hello Dão,

from https://hg.mozilla.org/mozilla-central/rev/fba4fab273da this issue should be fixed in latest Trunk 2017-09-28-10-01-23, but I still can reproduce the issue.

I am using a MacBook Air Non-Retina and macOS 10.12.6.

Can you still reproduce it too?

Comment 11

[Dão Gottwald [:dao]]

(In reply to Mehmet from comment #10)
> Hello Dão,
> 
> from https://hg.mozilla.org/mozilla-central/rev/fba4fab273da this issue
> should be fixed in latest Trunk 2017-09-28-10-01-23, but I still can
> reproduce the issue.
> 
> I am using a MacBook Air Non-Retina and macOS 10.12.6.

Could you please attach a screenshot?

> Can you still reproduce it too?

I'm not on Mac...

Comment 12

[Mehmet]

Attachment: Screenshot

(In reply to Dão Gottwald [::dao] from comment #11)
> (In reply to Mehmet from comment #10)
> > Hello Dão,
> > 
> > from https://hg.mozilla.org/mozilla-central/rev/fba4fab273da this issue
> > should be fixed in latest Trunk 2017-09-28-10-01-23, but I still can
> > reproduce the issue.
> > 
> > I am using a MacBook Air Non-Retina and macOS 10.12.6.
> 
> Could you please attach a screenshot?
> 
> > Can you still reproduce it too?
> 
> I'm not on Mac...

Of course. Please find it attached. Thanks.

Comment 13

[Mehmet]

I think you can leave it as it is. Adjusting it will probably cause a lot of work than that it brings any benefits. Thank you.

Comment 14

[Dão Gottwald [:dao]]

(In reply to Mehmet from comment #13)
> I think you can leave it as it is. Adjusting it will probably cause a lot of
> work than that it brings any benefits. Thank you.

Agreed. 5px at the top and 4px at the bottom means that if we move the text up further, it will be off-center again, just in the opposite direction.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 8912213 [details]
Bug 1399567 - Remove uneven vertical padding from urlbar and searchbar input boxes on Mac.

Approval Request Comment
[Feature/Bug causing the regression]: photon-visual polish
[User impact if declined]: location bar text is slightly misaligned vertically on Mac
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not formally yet, but see previous comments
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: just removing some obsolete Mac-specific CSS
[String changes made/needed]: /

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8912213 [details]
Bug 1399567 - Remove uneven vertical padding from urlbar and searchbar input boxes on Mac.

Polish photon, taking it.
Should be in 57b5

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/26f341039c20

Comment 18

[ovidiu boca[:Ovidiu]]

I will mark this as verified based on comment 13 and comment 14.