Increase vertical padding of toolbar buttons

VERIFIED FIXED in Firefox 55

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: nhnt11, Assigned: nhnt11)

Tracking

Trunk
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed, firefox57 verified)

Details

(Whiteboard: [photon-visual][p1][57])

Attachments

(1 attachment)

No description provided.
Iteration: --- → 55.5 - May 15
Flags: qe-verify?
Depends on: 1363840
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 on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

https://reviewboard.mozilla.org/r/138154/#review141668
Attachment #8866537 - Flags: review?(dao+bmo)
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 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.
Attachment #8866537 - Flags: review?(dao+bmo) → review-
Comment on attachment 8866537 [details]
Bug 1363869 - Increase vertical padding of toolbar buttons.

https://reviewboard.mozilla.org/r/138154/#review143026
Attachment #8866537 - Flags: review?(dao+bmo) → review+
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/738b18369999
Increase vertical padding of toolbar buttons. r=dao
Iteration: 55.5 - May 15 → 55.6 - May 29
Flags: qe-verify? → qe-verify+
QA Contact: brindusa.tot
https://hg.mozilla.org/mozilla-central/rev/738b18369999
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Blocks: 1365003
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.
Depends on: 1370401
Nihanth can you suggest me how to verify this bug? Thanks
Flags: needinfo?(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.
Flags: needinfo?(nhnt11)
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.