Width of the bookmark toolbar widget shrinks during animation of star

VERIFIED FIXED in Firefox 55

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: alice0775, Assigned: nhnt11)

Tracking

({regression})

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

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55 fixed)

Details

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

Attachments

(1 attachment)

Reporter

Description

2 years ago
Reproducible: always

Steps To Reproduce:
1. Click Star Icon

Actual Results:
Width of the bookmark toolbar widget shrinks during animation of star
So, Search bar shifts to the right.


Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=97a3a6e6550b0e5e6df000a0392e54fe5cc844e3&tochange=29d676d23989e8ebbc9ac3fa81bda4253798f5f5

Suspect:
4577d0417879	Nihanth Subramanya — Bug 1347543 - Use SVGs instead of PNGs for toolbar button icons. r=dao
Flags: needinfo?(nhnt11)
Component: Toolbars and Customization → Theme
OS: Windows 10 → All
Hardware: Unspecified → All
Whiteboard: [photon-visual][p1]
Flags: qe-verify+
No longer blocks: photon-visual
Priority: -- → P2
QA Contact: brindusa.tot

Updated

2 years ago
Depends on: 1362120
No longer depends on: 1362120
Assignee

Updated

2 years ago
Assignee: nobody → nhnt11
Flags: needinfo?(nhnt11)
Assignee

Updated

2 years ago
Status: NEW → ASSIGNED
Iteration: --- → 55.5 - May 15
Priority: P2 → P1
Comment hidden (mozreview-request)

Comment 2

2 years ago
mozreview-review
Comment on attachment 8864842 [details]
Bug 1362100 - Ensure bookmark toolbar button dropmarker icon remains correct size while animating.

https://reviewboard.mozilla.org/r/136538/#review139642

::: browser/themes/shared/bookmarked-notification.inc.css:59
(Diff revision 1)
>      background-image: url("chrome://browser/skin/places/bookmarks-notification-finish@2x.png");
>    }
>  }
>  
>  #bookmarks-menu-button[notification="finish"] > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon {
> -  list-style-image: none !important;
> +  visibility: none;

Does this even work? none (as opposed to hidden) isn't a valid visibility value.

If this works despite the invalid visibility value, perhaps we can completely remove this rule?
Attachment #8864842 - Flags: review?(dao+bmo) → review-
if you change that code, please test all the tier-1 platforms, IIRC there was something affecting one of the supported platforms more than the others, like overlapping dropdown images. It was long time ago, so I don't remember exactly, testing should remove any doubts.
(In reply to Marco Bonardo [::mak] from comment #3)
> if you change that code, please test all the tier-1 platforms, IIRC there
> was something affecting one of the supported platforms more than the others,
> like overlapping dropdown images. It was long time ago, so I don't remember
> exactly, testing should remove any doubts.

Things should now work consistently across platforms given bug 1352364.
Comment hidden (mozreview-request)
Assignee

Comment 6

2 years ago
mozreview-review-reply
Comment on attachment 8864842 [details]
Bug 1362100 - Ensure bookmark toolbar button dropmarker icon remains correct size while animating.

https://reviewboard.mozilla.org/r/136538/#review139642

> Does this even work? none (as opposed to hidden) isn't a valid visibility value.
> 
> If this works despite the invalid visibility value, perhaps we can completely remove this rule?

Oops, sorry, updated it. The logic was fine.

The width works fine without this rule; however, the icon that is overlayed on top of it grows during the animation (which is why, I suspect, that there is another one at all - to prevent the button from growing during the animation as opposed to just the icon). When the icon above it grows, the one below is visible under it and looks... bad.

Comment 7

2 years ago
mozreview-review
Comment on attachment 8864842 [details]
Bug 1362100 - Ensure bookmark toolbar button dropmarker icon remains correct size while animating.

https://reviewboard.mozilla.org/r/136538/#review139650

::: browser/themes/shared/bookmarked-notification.inc.css:59
(Diff revision 2)
>      background-image: url("chrome://browser/skin/places/bookmarks-notification-finish@2x.png");
>    }
>  }
>  
>  #bookmarks-menu-button[notification="finish"] > .toolbarbutton-menubutton-dropmarker > .dropmarker-icon {
> -  list-style-image: none !important;
> +  visibility: hidden;

This removes the button styling during the animation. I'd suggest using fill: transparent; instead.
Attachment #8864842 - Flags: review?(dao+bmo) → review-
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8864842 [details]
Bug 1362100 - Ensure bookmark toolbar button dropmarker icon remains correct size while animating.

https://reviewboard.mozilla.org/r/136538/#review139656
Attachment #8864842 - Flags: review?(dao+bmo) → review+

Comment 11

2 years ago
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3679236300ee
Ensure bookmark toolbar button dropmarker icon remains correct size while animating. r=dao

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3679236300ee
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I have reproduced this bug with nightly 55.0a1 (2017-05-04) on Ubuntu 16.04(64 Bit).

The bug's fix is now verified on Latest nightly 55.0a1.

Build ID   : 20170518100156

User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

[Bugday-20170517]
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.