Hover feedback for zoom reset button is too tall

VERIFIED FIXED in Firefox 53

Status

()

Firefox
Theme
VERIFIED FIXED
10 months ago
8 months ago

People

(Reporter: bogdan_maris, Assigned: dao)

Tracking

({regression})

Trunk
Firefox 54
All
Mac OS X
regression
Points:
---

Firefox Tracking Flags

(firefox51 unaffected, firefox52 unaffected, firefox53 fixed, firefox54 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

10 months ago
Created attachment 8838089 [details]
Screencast showing the issue

[Affected versions]:
- latest Developer Edition 53.0a2
- latest Nightly 54.0a1

[Affected platforms]:
- Mac OS X 10.11.6
- macOS 10.12.3

[Unaffected platforms]:
- Windows 10 64bit
- Ubuntu 16.04 32bit

[Steps to reproduce]:
1. Start Firefox
2. Enter Customization and drag the zoom buttons to ToolBar
3. Hover over 100% value

[Expected result]:
- The hover effect is the same as on the other buttons.

[Actual result]:
- The hover effect for 100% button is different (larger height) than the other buttons.

[Regression range]:
- This is a recent regression, it
Last good revision: 42086c06f756cda7fbc25a2e7c20a5711f7e5f26 (2016-12-12)
First bad revision: f46f85dcfbc2b3098ea758825d18be6fab33cbc6 (2016-12-13)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=42086c06f756cda7fbc25a2e7c20a5711f7e5f26&tochange=f46f85dcfbc2b3098ea758825d18be6fab33cbc6

Most likely caused by:
8bedb73fd7ec	Dão Gottwald — Bug 1322430 - Clean up toolbar button margin and padding rules. r=gijs

[Additional notes]:
- This is also repro with Compact themes.
- Screencast showing the issue attached.
- Not reproducible using Fx 52 beta 6 with Dev Edition theme installed.
(Reporter)

Updated

10 months ago
Flags: needinfo?(dao+bmo)
(Assignee)

Updated

10 months ago
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 1

10 months ago
After taking a closer look at Ubuntu, there is no highlight effect for 100% button in default theme, on compact themes only the color of the number will change, not the same highlight effect as on other buttons. Also the toolbar shakes when clicking any of Zoom buttons. Let me know if I should log a new bug for Linux behavior.
Here is a screencast from Ubuntu: https://dl.dropboxusercontent.com/u/109148197/Screencast%202017-02-16%2018%3A05%3A02.mp4
Comment hidden (mozreview-request)
(Assignee)

Comment 3

10 months ago
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #1)
> After taking a closer look at Ubuntu, there is no highlight effect for 100%
> button in default theme, on compact themes only the color of the number will
> change, not the same highlight effect as on other buttons. Also the toolbar
> shakes when clicking any of Zoom buttons. Let me know if I should log a new
> bug for Linux behavior.

The above would be a separate bug / regression since bug 1322430 was limited to Mac.
Summary: Highlight effect for 100% zoom button larger then all highlight effect of other buttons → Hover feedback for zoom reset button is too tall

Comment 4

10 months ago
mozreview-review
Comment on attachment 8838105 [details]
Bug 1340173 - Stop removing the zoom reset button's vertical margin.

https://reviewboard.mozilla.org/r/113090/#review114560

Redirecting to Gijs as he reviewed the previous patch in this series.
Attachment #8838105 - Flags: review?(jaws)
Attachment #8838105 - Flags: review?(gijskruitbosch+bugs)

Comment 5

10 months ago
(In reply to Dão Gottwald [:dao] from comment #3)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #1)
> > After taking a closer look at Ubuntu, there is no highlight effect for 100%
> > button in default theme, on compact themes only the color of the number will
> > change, not the same highlight effect as on other buttons. Also the toolbar
> > shakes when clicking any of Zoom buttons. Let me know if I should log a new
> > bug for Linux behavior.
> 
> The above would be a separate bug / regression since bug 1322430 was limited
> to Mac.

Bogdan can you make sure this gets filed? Thank you.
Flags: needinfo?(bogdan.maris)

Comment 6

10 months ago
mozreview-review
Comment on attachment 8838105 [details]
Bug 1340173 - Stop removing the zoom reset button's vertical margin.

https://reviewboard.mozilla.org/r/113090/#review114574
Attachment #8838105 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 7

10 months ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/00fd29fee54e
Stop removing the zoom reset button's vertical margin. r=Gijs

Comment 8

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/00fd29fee54e
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox54: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Please request Aurora approval on this when you get a chance.
Blocks: 1322430
No longer depends on: 1322430
Flags: needinfo?(dao+bmo)
(Reporter)

Comment 10

10 months ago
(In reply to :Gijs from comment #5)
> (In reply to Dão Gottwald [:dao] from comment #3)
> > (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #1)
> > > After taking a closer look at Ubuntu, there is no highlight effect for 100%
> > > button in default theme, on compact themes only the color of the number will
> > > change, not the same highlight effect as on other buttons. Also the toolbar
> > > shakes when clicking any of Zoom buttons. Let me know if I should log a new
> > > bug for Linux behavior.
> > 
> > The above would be a separate bug / regression since bug 1322430 was limited
> > to Mac.
> 
> Bogdan can you make sure this gets filed? Thank you.

Sure, I actually logged two bugs (bug 1340435 and bug 1340436), one for the hover feedback and one for the moving toolbar since mozregression pointed to two different regression ranges. I've CC'ed you and Dão on both of them.
Flags: needinfo?(bogdan.maris)
(Assignee)

Comment 11

10 months ago
Comment on attachment 8838105 [details]
Bug 1340173 - Stop removing the zoom reset button's vertical margin.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1322430
[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]: no
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: trivial CSS patch
[String changes made/needed]: /
Flags: needinfo?(dao+bmo)
Attachment #8838105 - Flags: approval-mozilla-aurora?

Comment 12

10 months ago
Hi Bogdan,
could you help verify if this issue is fixed as expected on the latest Nightly build? Thanks!
Flags: needinfo?(bogdan.maris)
(Reporter)

Comment 13

10 months ago
I can confirm that the hover feedback has the same height as other buttons bug I can see now that the hover feedback is not aligned with the separator between zoom out and zoom reset. (This can also be seen on Firefox 52 beta and 51 RC). I should point out that this thing also happened before this patch landed.

Should I log a new bug on this?
Flags: needinfo?(bogdan.maris) → needinfo?(dao+bmo)
(Assignee)

Comment 14

10 months ago
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #13)
> I can confirm that the hover feedback has the same height as other buttons
> bug I can see now that the hover feedback is not aligned with the separator
> between zoom out and zoom reset. (This can also be seen on Firefox 52 beta
> and 51 RC). I should point out that this thing also happened before this
> patch landed.
> 
> Should I log a new bug on this?

Yes, please.
Flags: needinfo?(dao+bmo)

Comment 15

10 months ago
Comment on attachment 8838105 [details]
Bug 1340173 - Stop removing the zoom reset button's vertical margin.

Fix a UI issue and was verified. Aurora53+.
Attachment #8838105 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Reporter)

Comment 16

10 months ago
(In reply to Dão Gottwald [:dao] from comment #14)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #13)
> > I can confirm that the hover feedback has the same height as other buttons
> > bug I can see now that the hover feedback is not aligned with the separator
> > between zoom out and zoom reset. (This can also be seen on Firefox 52 beta
> > and 51 RC). I should point out that this thing also happened before this
> > patch landed.
> > 
> > Should I log a new bug on this?
> 
> Yes, please.

Logged bug 1341227 on this. Also marking as verified fixed on latest Nightly 54.0a1.
status-firefox54: fixed → verified

Comment 17

10 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/c0053a0cf40f
status-firefox53: affected → fixed
Comment hidden (obsolete)

Comment 19

8 months ago
Status: FIXED & VERIFIED.
Browser: Firefox Aurora 54

The issue is no longer reproducible on Firefox Aurora 
Test were done under Windows 10 x64.

[bugday-20170419]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.