1340436 - [Linux] Clicking on zoom buttons positioned in Toolbar makes it move a few pixels to the left

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Bogdan Maris [:bogdan_maris], Release Desktop QA]

Attachment: Screencast showing the issue

[Affected versions]:
- Firefox 51.0.1 RC
- Firefox 52 beta 6
- latest Developer Edition 53.0a2
- latest Nightly 54.0a1

[Affected platforms]:
- Ubuntu 16.04 32bit

[Unaffected platforms]:
- Windows 10 64bit
- macOS 10.12.3

[Steps to reproduce]:
1. Start Firefox
2. Enter Customization and drag the zoom buttons to ToolBar
3. Click on any zoom button.

[Expected result]:
- Toolbar does not move when hitting zoom buttons.

[Actual result]:
- Toolbar moves a few pixels to the left when clicking on zoom buttons if buttons are placed in the toolbar..

[Regression range]:
- This is NOT a recent regression (but an old one), I've tracked it back to Nightly to 2015-08-28:
Last good revision: f8086bd3c84fc1a42c3625cf3cc2253f0a5e8cfd (2015-08-27)
First bad revision: 87e23922be375985d0b1906ed5ba5f095f323a38 (2015-08-28)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f8086bd3c84fc1a42c3625cf3cc2253f0a5e8cfd&tochange=87e23922be375985d0b1906ed5ba5f095f323a38

Not sure which bug caused this but I see some bugs that Dão worked on and are related to theme. 

[Additional notes]:
- Screencast showing the issue attached.
- I marked this as a regression even though it's a very old one, feel free to remove the regression keyword if you feel this is not correct.
- If for some reason the attachment says the file is corrupt please visit this dropbox link for the same video: https://dl.dropboxusercontent.com/u/109148197/Screencast%202017-02-16%2018%3A05%3A02.mp4

Comment 2

[Dão Gottwald [::dao]]

Attachment: Bug 1340436 - Consolidate .toolbarbutton-1 padding rules and remove the unneeded vertical padding.

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

Comment 3

[:Gijs (he/him)]

Comment on attachment 8838479 [details]
Bug 1340436 - Consolidate .toolbarbutton-1 padding rules and remove the unneeded vertical padding.

https://reviewboard.mozilla.org/r/113416/#review114868

LGTM

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5499745680e0
Consolidate .toolbarbutton-1 padding rules and remove the unneeded vertical padding. r=Gijs

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5499745680e0

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/Beta approval on this when you get a chance.

Comment 7

[Dão Gottwald [::dao]]

I'll let this bake for a week before I'll consider uplifting it.

Comment 8

[Dão Gottwald [::dao]]

Comment on attachment 8838479 [details]
Bug 1340436 - Consolidate .toolbarbutton-1 padding rules and remove the unneeded vertical padding.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1198703
[User impact if declined]: see comment 0
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: I think this could do without a manual test (steps are in comment 0)
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: slightly risky
[Why is the change risky/not risky?]: the change applies to all toolbar buttons in the navigation and tab toolbars, which could have unforeseen consequences, but at this point I'm sure they'd be minor since this has baked for a week
[String changes made/needed]: /

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Is this wontfix for 52?

Comment 10

[Gerry Chang [:gchang]]

Hi :bogdan_maris,
could you help verify if this issue is fixed as expected on the latest Nightly build? Thanks!

Comment 11

[Dão Gottwald [::dao]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> Is this wontfix for 52?

Too late for 52, I think.

Comment 12

[Bogdan Maris [:bogdan_maris], Release Desktop QA]

(In reply to Gerry Chang [:gchang] from comment #10)
> Hi :bogdan_maris,
> could you help verify if this issue is fixed as expected on the latest
> Nightly build? Thanks!

Looks good to me using latest Nightly 54.0a1 from today.

Comment 13

[Gerry Chang [:gchang]]

Comment on attachment 8838479 [details]
Bug 1340436 - Consolidate .toolbarbutton-1 padding rules and remove the unneeded vertical padding.

Polish an UI issue in toolbar and was verified. Aurora53+.

Comment 14

[Dão Gottwald [::dao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/95d5cceb04c19391e691937194f25baaf0ee4dcf