Bug 1383879 - Hide the toolbarbutton-animatable-box for animations when they are not playing to prevent weird layout issues due to position:relative not working as expected in XUL layout. r?dao draft
authorJared Wein <jwein@mozilla.com>
Thu, 27 Jul 2017 16:00:08 -0400
changeset 618475 ffc668b977851384cf3d67fae3fe1a5366a80c3c
parent 617134 a4afa89bfdd10a903cfa9aa5a5bd1624dc85548c
child 640083 8031d8846b666ce915013ed248880251230e3cf5
push id71348
push userbmo:jaws@mozilla.com
push dateMon, 31 Jul 2017 15:15:31 +0000
reviewersdao
bugs1383879
milestone56.0a1
Bug 1383879 - Hide the toolbarbutton-animatable-box for animations when they are not playing to prevent weird layout issues due to position:relative not working as expected in XUL layout. r?dao I will file a separate bug to get help from some layout peers if this patch is accepted. MozReview-Commit-ID: 6T9rRCxMbYr
browser/base/content/browser-places.js
browser/extensions/pocket/skin/shared/pocket.css
browser/themes/shared/toolbarbutton-icons.inc.css
browser/themes/shared/urlbar-searchbar.inc.css
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -1710,30 +1710,25 @@ var BookmarkingUI = {
     if (!AppConstants.MOZ_PHOTON_THEME) {
       this._updateCustomizationState();
     }
 
     if (AppConstants.MOZ_PHOTON_ANIMATIONS &&
         Services.prefs.getBoolPref("toolkit.cosmeticAnimations.enabled")) {
       let starButtonBox = document.getElementById("star-button-box");
       starButtonBox.setAttribute("animationsenabled", "true");
-      this.star.addEventListener("mouseover", this, {once: true});
     }
   },
 
   _hasBookmarksObserver: false,
   _itemGuids: new Set(),
   uninit: function BUI_uninit() {
     this.updateBookmarkPageMenuItem(true);
     CustomizableUI.removeListener(this);
 
-    if (AppConstants.MOZ_PHOTON_ANIMATIONS) {
-      this.star.removeEventListener("mouseover", this);
-    }
-
     this._uninitView();
 
     if (this._hasBookmarksObserver) {
       PlacesUtils.bookmarks.removeObserver(this);
     }
 
     if (this._pendingUpdate) {
       delete this._pendingUpdate;
@@ -1953,21 +1948,16 @@ var BookmarkingUI = {
   },
 
   onCurrentPageContextPopupShowing() {
     this.updateBookmarkPageMenuItem();
   },
 
   handleEvent: function BUI_handleEvent(aEvent) {
     switch (aEvent.type) {
-      case "mouseover":
-        if (AppConstants.MOZ_PHOTON_ANIMATIONS) {
-          this.star.setAttribute("preloadanimations", "true");
-        }
-        break;
       case "ViewShowing":
         this.onPanelMenuViewShowing(aEvent);
         break;
       case "ViewHiding":
         this.onPanelMenuViewHiding(aEvent);
         break;
     }
   },
--- a/browser/extensions/pocket/skin/shared/pocket.css
+++ b/browser/extensions/pocket/skin/shared/pocket.css
@@ -40,46 +40,46 @@ toolbar[brighttext] #pocket-button {
   from {
     transform: scaleX(-1) translateX(0);
   }
   to {
     transform: scaleX(-1) translateX(-220px);
   }
 }
 
-#pocket-button > .toolbarbutton-animatable-box {
+#pocket-button[open="true"][animationsenabled] > .toolbarbutton-animatable-box {
   position: absolute;
   overflow: hidden;
   top: calc(50% - 9px); /* 9px is half the height of the sprite */
   /* Since .toolbarbutton-icon uses a different width than the animatable box,
      we need to set a padding relative to the difference in widths. */
   margin-inline-start: calc((16px + 2 * var(--toolbarbutton-inner-padding) - 20px) / 2);
   /* Set the min- and max- width and height of the box equal to that
      of each frame of the SVG sprite. Setting the width and height via
      the `width` and `height` CSS properties causes an assertion for
      `inline-size less than zero: 'aContainingBlockISize >= 0'` (bug 1379332). */
   min-width: 20px;
   max-width: 20px;
   min-height: 18px;
   max-height: 18px;
 }
 
-#pocket-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
+#pocket-button[open="true"][animationsenabled] > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   height: var(--toolbarbutton-height); /* Height must be equal to height of toolbarbutton padding-box */
 }
 
 #pocket-button[open="true"][animationsenabled][cui-areatype="toolbar"]:not([overflowedItem="true"]) {
   position: relative;
 }
 
 /* Preload pocket-animation.svg and library-pocket-animation.svg to prevent
    a flicker at the start of either animation. The preloading of the library
    animation is triggered off of hovering the pocket button since the pocket
    button always animates before the library button. */
-#pocket-button[animationsenabled][cui-areatype="toolbar"]:not([overflowedItem="true"]):not([open="true"]):hover > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
+#pocket-button[animationsenabled][cui-areatype="toolbar"]:not([overflowedItem="true"]):not([open="true"]):hover {
   background-image: url("chrome://pocket-shared/skin/pocket-animation.svg"),
                     url("chrome://pocket-shared/skin/library-pocket-animation.svg");
   background-size: 0, 0;
 }
 
 #pocket-button[open="true"][animationsenabled][cui-areatype="toolbar"]:not([overflowedItem="true"]) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   animation-name: pocket-animation;
   animation-timing-function: steps(11);
@@ -130,17 +130,17 @@ toolbar[brighttext] #pocket-button {
   }
 }
 
 /* The animations for the pocket-button and library-button are disabled
    outside of the nav-bar due to bug 1382894. */
 :-moz-any(#pocket-button, #library-button) > .toolbarbutton-animatable-box {
   display: none;
 }
-#nav-bar :-moz-any(#pocket-button, #library-button) > .toolbarbutton-animatable-box {
+#nav-bar :-moz-any(#pocket-button[open="true"][animationsenabled], #library-button[animate="pocket"]) > .toolbarbutton-animatable-box {
   display: -moz-box;
 }
 
 /* We need to use an animation here instead of a transition
    to guarantee that the animation succeeds. With transitions
    if the starting value is already equal to the end value
    then no transition will occur and thus no transitionend event. */
 @keyframes library-pocket-fade {
--- a/browser/themes/shared/toolbarbutton-icons.inc.css
+++ b/browser/themes/shared/toolbarbutton-icons.inc.css
@@ -80,80 +80,73 @@ toolbar[brighttext] .toolbarbutton-1 {
   from {
     transform: scaleX(-1) translateX(0);
   }
   to {
     transform: scaleX(-1) translateX(-450px);
   }
 }
 
+#stop-reload-button:not([animate]) > #reload-button:not([displaystop]) > .toolbarbutton-animatable-box,
+#stop-reload-button:not([animate]) > #reload-button[displaystop] + #stop-button > .toolbarbutton-animatable-box {
+  display: none;
+}
+
 #stop-reload-button[animate] > #reload-button,
 #stop-reload-button[animate] > #stop-button {
   position: relative;
 }
 
-#reload-button > .toolbarbutton-animatable-box,
-#stop-button > .toolbarbutton-animatable-box {
+#stop-reload-button[animate] > #reload-button:not([displaystop]) > .toolbarbutton-animatable-box,
+#stop-reload-button[animate] > #reload-button[displaystop] + #stop-button > .toolbarbutton-animatable-box {
   position: absolute;
   overflow: hidden;
   top: calc(50% - 10px); /* Vertically center the 20px tall animatable image */
   /* Since .toolbarbutton-icon uses a different width than the animatable-box,
      we need to set a padding relative to the difference in widths. */
   margin-inline-start: calc((16px + 2 * var(--toolbarbutton-inner-padding) - 18px) / 2);
   /* Set the min- and max- width and height of the box equal to that
      of each frame of the SVG sprite. Setting the width and height via
      the `width` and `height` CSS properties causes an assertion for
      `inline-size less than zero: 'aContainingBlockISize >= 0'` (bug 1379332). */
   min-width: 18px;
   max-width: 18px;
   min-height: 20px;
   max-height: 20px;
 }
 
-#reload-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image,
-#stop-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
+#stop-reload-button[animate] > #reload-button:not([displaystop]) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image,
+#stop-reload-button[animate] > #reload-button[displaystop] + #stop-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   height: var(--toolbarbutton-height); /* Height must be equal to height of toolbarbutton padding-box */
 }
 
-#stop-reload-button[animate] > #reload-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
-  background-image: url("chrome://browser/skin/reload-to-stop.svg");
-  width: 468px;
-}
-
 #stop-reload-button[animate] > #reload-button:not([displaystop]) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   animation-name: reload-to-stop;
+  animation-timing-function: steps(25);
+  animation-duration: 400ms;
+  background-image: url("chrome://browser/skin/reload-to-stop.svg");
+  width: 468px;
 }
 
 #stop-reload-button[animate] > #reload-button:not([displaystop]):-moz-locale-dir(rtl) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   animation-name: reload-to-stop-rtl;
 }
 
-#reload-button:not([displaystop]) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
+#stop-reload-button[animate] > #reload-button[displaystop] + #stop-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
+  background-image: url("chrome://browser/skin/stop-to-reload.svg");
+  width: 468px;
+  animation-name: stop-to-reload;
   animation-timing-function: steps(25);
   animation-duration: 400ms;
 }
 
-#stop-reload-button[animate] > #reload-button[displaystop] + #stop-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
-  background-image: url("chrome://browser/skin/stop-to-reload.svg");
-  width: 468px;
-}
-
-#stop-reload-button[animate] > #reload-button[displaystop] + #stop-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
-  animation-name: stop-to-reload;
-}
-
 #stop-reload-button[animate] > #reload-button[displaystop] + #stop-button:-moz-locale-dir(rtl) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   animation-name: stop-to-reload-rtl;
 }
 
-#reload-button[displaystop] + #stop-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
-  animation-timing-function: steps(25);
-  animation-duration: 400ms;
-}
-
 #reload-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   transform: translateX(-450px);
 }
 
 #reload-button:-moz-locale-dir(rtl) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   transform: scaleX(-1) translateX(-450px);
 }
 
@@ -364,39 +357,41 @@ toolbar:not([brighttext]) #bookmarks-men
     fill: #30A3FF;
   }
   to {
     fill: inherit;
   }
 }
 
 #nav-bar-overflow-button > .toolbarbutton-animatable-box {
+  display: none;
+}
+
+#nav-bar-overflow-button[animate] > .toolbarbutton-animatable-box {
+  display: -moz-box;
   position: absolute;
   overflow: hidden;
   top: calc(50% - 18px); /* Vertically center the 36px tall animatable image */
   /* Since .toolbarbutton-icon uses a different width than the animatable box,
      we need to set a padding relative to the difference in widths. */
   margin-inline-start: calc((16px + 2 * var(--toolbarbutton-inner-padding) - 18px) / 2);
   width: 18px; /* Width of each frame within the SVG sprite */
   height: 36px; /* Height of each frame within the SVG sprite */
 }
 
-#nav-bar-overflow-button > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
-  /* Height must be equal to height of toolbarbutton padding-box. --toolbarbutton-height
-     is calculated and set during runtime by BrowserUtils.setToolbarButtonHeightProperty()
-     before the animation begins. */
-  height: var(--toolbarbutton-height);
-}
-
 #nav-bar-overflow-button[animate] > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   animation-name: overflow-animation;
   animation-timing-function: steps(70);
   animation-duration: 1.1s;
   background-image: url("chrome://browser/skin/chevron-animation.svg");
   width: 1278px;
+  /* Height must be equal to height of toolbarbutton padding-box. --toolbarbutton-height
+     is calculated and set during runtime by BrowserUtils.setToolbarButtonHeightProperty()
+     before the animation begins. */
+  height: var(--toolbarbutton-height);
 }
 
 #nav-bar-overflow-button[animate]:-moz-locale-dir(rtl) > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   animation-name: overflow-animation-rtl;
 }
 
 #nav-bar-overflow-button[animate][fade] > .toolbarbutton-animatable-box > .toolbarbutton-animatable-image {
   animation-name: overflow-fade;
@@ -473,25 +468,30 @@ toolbar:not([brighttext]) #bookmarks-men
   from {
     fill: rgb(10,132,255);
   }
   to {
     fill: inherit;
   }
 }
 
+#library-button > .toolbarbutton-animatable-box {
+  display: none;
+}
+
 #library-button[animate="bookmark"] {
   position: relative;
 }
 
 #library-button[animate="bookmark"] > .toolbarbutton-icon {
   fill: transparent;
 }
 
 #library-button[animate="bookmark"] > .toolbarbutton-animatable-box {
+  display: -moz-box;
   position: absolute;
   overflow: hidden;
   top: calc(50% - 27px); /* 27px is half the height of the sprite */
   /* Set a margin relative to the difference in widths of the .toolbarbutton-icon
      and the .toolbar-animatable-box */
   margin-inline-start: calc((16px + 2 * var(--toolbarbutton-inner-padding) - 22px) / 2);
   /* Set the min- and max- width and height of the box equal to that
      of each frame of the SVG sprite (must use min- and max- due to bug 1379332). */
--- a/browser/themes/shared/urlbar-searchbar.inc.css
+++ b/browser/themes/shared/urlbar-searchbar.inc.css
@@ -118,17 +118,17 @@
   list-style-image: url("chrome://browser/skin/bookmark.svg");
   fill-opacity: 1;
   fill: var(--toolbarbutton-icon-fill-attention);
 }
 
 %ifdef MOZ_PHOTON_ANIMATIONS
 /* Preload the bookmark animations to prevent a flicker during the first playing
    of the animations. */
-#star-button[preloadanimations] + #star-button-animatable-box > #star-button-animatable-image {
+#star-button:hover + #star-button-animatable-box > #star-button-animatable-image {
   background-image: url("chrome://browser/skin/bookmark-animation.svg"),
                     url("chrome://browser/skin/library-bookmark-animation.svg");
   background-size: 0, 0;
 }
 
 #star-button-box[animationsenabled] > #star-button[starred][animate] {
   fill: transparent;
   position: relative;