1362100 - Width of the bookmark toolbar widget shrinks during animation of star

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Alice0775 White]

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

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1362100 - Ensure bookmark toolbar button dropmarker icon remains correct size while animating.

The actual dropmarker's list-style-image was forced to none while animating, with another one overlayed on top of it.
Setting visibility: none; accomplishes the goal of hiding the default icon without affecting layout.

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

Comment 2

[Dão Gottwald [:dao]]

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/[e-mail]");
>    }
>  }
>  
>  #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?

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

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.

Comment 4

[Dão Gottwald [:dao]]

(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 5

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136538/diff/1-2/

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

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

[Dão Gottwald [:dao]]

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/[e-mail]");
>    }
>  }
>  
>  #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.

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136538/diff/2-3/

Comment 9

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136538/diff/3-4/

Comment 10

[Dão Gottwald [:dao]]

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3679236300ee
Ensure bookmark toolbar button dropmarker icon remains correct size while animating. r=dao

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/3679236300ee

Comment 13

[Md. Majedul islam]

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]