Closed Bug 1362100 Opened 3 years ago Closed 3 years ago

Width of the bookmark toolbar widget shrinks during animation of star

Categories

(Firefox :: Theme, defect, P1)

55 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.5 - May 15
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: alice0775, Assigned: nhnt11)

References

Details

(Keywords: regression, Whiteboard: [photon-visual][p1])

Attachments

(1 file)

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
Depends on: 1362120
No longer depends on: 1362120
Assignee: nobody → nhnt11
Flags: needinfo?(nhnt11)
Status: NEW → ASSIGNED
Iteration: --- → 55.5 - May 15
Priority: P2 → P1
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 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 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 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+
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
https://hg.mozilla.org/mozilla-central/rev/3679236300ee
Status: ASSIGNED → RESOLVED
Closed: 3 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.