1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, enhancement, P1)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Bug 1363485 will be adding the capability for moving the stop/reload button to an arbitrary location. However due to bug 1382894 we cannot run our animations outside of the nav-bar.

This bug is on file to disable the animations for the stop/reload button if they are not in the nav-bar.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

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

Comment 2

[:Gijs (he/him)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

https://reviewboard.mozilla.org/r/159570/#review165094

rs=me

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

https://reviewboard.mozilla.org/r/159570/#review165132

::: browser/themes/shared/toolbarbutton-icons.inc.css:52
(Diff revision 1)
> +/* The animations for the reload-button and stop-button are disabled
> +   outside of the nav-bar due to bug 1382894. */
> +:-moz-any(#reload-button, #stop-button) > .toolbarbutton-animatable-box {
> +  display: none;
> +}
> +#nav-bar :-moz-any(#reload-button, #stop-button) > .toolbarbutton-animatable-box {

Please use #nav-bar-customization-target and the child selector here.

Comment 4

[Dão Gottwald [:dao]]

This should land after bug 1363485.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

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

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

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

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

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

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

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

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

Gijs, can you please re-review this patch? I had to add some code to browser.js to not add the "animate" attribute if the button was placed outside of the nav-bar. 

Without this code, if the button was placed in the tabs toolbar, next a page refreshed, then the button moved to the nav-bar, it would have the animate="true" attribute when no page load is occurring.

Comment 10

[:Gijs (he/him)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

https://reviewboard.mozilla.org/r/159570/#review169812

Good catch, thanks!

Comment 11

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s c268ecb818a9 -d db11ec58efb6: rebasing 411386:c268ecb818a9 "Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894. r=Gijs" (tip)
merging browser/base/content/browser.js
warning: conflicts while merging browser/base/content/browser.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159570/diff/5-6/

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8888586 [details]
Bug 1382898 - Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159570/diff/5-6/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdc0c0802e54
Disable the Stop / Reload animation if the stop / reload buttons are placed outside of the nav-bar due to bug 1382894. r=Gijs

Comment 15

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

https://hg.mozilla.org/mozilla-central/rev/cdc0c0802e54

Comment 16

[Stefan [:StefanG_QA]]

User Agent    Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build   ID    20170905100117

This bug fix is Verified with latest Nightly 57.0a1.

The the stop / reload  button animation is disabled when the button is placed outside of the nav-bar.