1363869 - Increase vertical padding of toolbar buttons

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1363869 - Increase vertical padding of toolbar buttons.

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

Comment 2

[Dão Gottwald [::dao]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

https://reviewboard.mozilla.org/r/138154/#review141314

--toolbarbutton-vertical-inner-padding is used in quite a few more places than this patch touches. Shouldn't they all switch to --toolbarbutton-inner-padding for photon?

Comment 3

[Dão Gottwald [::dao]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

https://reviewboard.mozilla.org/r/138154/#review141668

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

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

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

https://reviewboard.mozilla.org/r/138154/#review141314

So, to make it a bit simpler, I changed the name of the variable to just --toolbarbutton-inner-padding, and put the two different values (before/after photon) in ifdefs. What do you think? I figured this is cleaner than a bunch of ifdefs everywhere.

Comment 6

[Dão Gottwald [::dao]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

https://reviewboard.mozilla.org/r/138154/#review143024

::: browser/extensions/pocket/skin/osx/pocket.css:4
(Diff revision 2)
>  @import url("chrome://pocket-shared/skin/pocket.css");
>  
>  #nav-bar #pocket-button > .toolbarbutton-icon {
> +%ifdef MOZ_PHOTON_THEME

I don't think this file is currently preprocessed, so this doesn't work.

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

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

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/138154/diff/3-4/

Comment 9

[Dão Gottwald [::dao]]

Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

https://reviewboard.mozilla.org/r/138154/#review143026

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/738b18369999
Increase vertical padding of toolbar buttons. r=dao

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/738b18369999

Comment 12

[Johann Hofmann [:johannh]]

Screenshots:

https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=b8e9b674033bcd1f3a4c59b9d0ee7619c1a17cc5&newProject=mozilla-central&newRev=985b3ee939338022ef44028b5251f77af19c3638

Seems to be missing screenshots from Windows, unfortunately (and OSX has been gone for a couple of days, too). Should be fine, though.

Comment 13

[ovidiu boca[:Ovidiu]]

Nihanth can you suggest me how to verify this bug? Thanks

Comment 14

[(inactive) Nihanth Subramanya [:nhnt11]]

(In reply to ovidiu boca[:Ovidiu] from comment #13)
> Nihanth can you suggest me how to verify this bug? Thanks

Bah, I forgot to reply to this. I've been in touch with Brindusa on other channels though, here's the gist:

This bug adds padding to the top and bottom of toolbar button icons. Before this landed, the buttons were rectangular. After this landed, they should be square.

Comment 15

[ovidiu boca[:Ovidiu]]

I verified this on Mac OS X 10.10, Mac OS X 10.12, Windows 10, Windows 7, Ubuntu 16.04 with the latest Nightly 57.0a1(2017-08-15) and I can confirm the fix, the buttons are square.