Bug 1384341 - Set a minimum size for the animation area so animations that are larger than default-sized (non-compact) toolbar buttons don't get clipped. Also, don't apply z-index:2 to the selected tab in photon builds since the tabs won't be overlapping each other in photon. The z-index was causing the top part of the animation to get clipped by the selected tab. r?mconley draft
authorJared Wein <jwein@mozilla.com>
Wed, 26 Jul 2017 14:20:28 -0400
changeset 616109 c321f260e2f06a2680890030ef481ff90bf67c93
parent 615176 32d9d1e81cc607320a36391845917f645f7a7f72
child 639384 ff41a87e0b8fb977c1a60ebe3d65e9f0acb1e404
push id70590
push userbmo:jaws@mozilla.com
push dateWed, 26 Jul 2017 18:25:54 +0000
reviewersmconley
bugs1384341
milestone56.0a1
Bug 1384341 - Set a minimum size for the animation area so animations that are larger than default-sized (non-compact) toolbar buttons don't get clipped. Also, don't apply z-index:2 to the selected tab in photon builds since the tabs won't be overlapping each other in photon. The z-index was causing the top part of the animation to get clipped by the selected tab. r?mconley MozReview-Commit-ID: 2n141CGVogi
browser/base/content/browser-places.js
browser/extensions/pocket/content/Pocket.jsm
browser/themes/shared/tabs.inc.css
toolkit/modules/BrowserUtils.jsm
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -132,17 +132,17 @@ var StarUI = {
                               .transact().catch(Cu.reportError);
           } else if (this._isNewBookmark &&
                      Services.prefs.getBoolPref("toolkit.cosmeticAnimations.enabled") &&
                      AppConstants.MOZ_PHOTON_ANIMATIONS &&
                      (libraryButton = document.getElementById("library-button")) &&
                      libraryButton.getAttribute("cui-areatype") != "menu-panel" &&
                      libraryButton.getAttribute("overflowedItem") != "true" &&
                      libraryButton.closest("#nav-bar")) {
-            BrowserUtils.setToolbarButtonHeightProperty(libraryButton);
+            BrowserUtils.setToolbarButtonHeightProperty(libraryButton, {minimumHeight: 38});
             libraryButton.removeAttribute("fade");
             libraryButton.setAttribute("animate", "bookmark");
             libraryButton.addEventListener("animationend", this);
           }
         }
         break;
       }
       case "keypress":
@@ -1938,17 +1938,17 @@ var BookmarkingUI = {
     // Ignore clicks on the star if we are updating its state.
     if (!this._pendingUpdate) {
       let isBookmarked = this._itemGuids.size > 0;
       // Disable the old animation in photon
       if (!isBookmarked && !AppConstants.MOZ_PHOTON_THEME)
         this._showBookmarkedNotification();
       // Set up variables for new animation in Photon
       if (!isBookmarked && AppConstants.MOZ_PHOTON_ANIMATIONS) {
-        BrowserUtils.setToolbarButtonHeightProperty(this.star);
+        BrowserUtils.setToolbarButtonHeightProperty(this.star, {minimumHeight: 38});
         this.star.setAttribute("animate", "true");
       }
       PlacesCommandHook.bookmarkCurrentPage(true);
     }
   },
 
   onCurrentPageContextPopupShowing() {
     this.updateBookmarkPageMenuItem();
--- a/browser/extensions/pocket/content/Pocket.jsm
+++ b/browser/extensions/pocket/content/Pocket.jsm
@@ -30,17 +30,17 @@ var Pocket = {
 
   onPanelViewShowing(event) {
     let document = event.target.ownerDocument;
     let window = document.defaultView;
     let iframe = window.pktUI.getPanelFrame();
 
     let libraryButton = document.getElementById("library-button");
     if (libraryButton) {
-      BrowserUtils.setToolbarButtonHeightProperty(libraryButton);
+      BrowserUtils.setToolbarButtonHeightProperty(libraryButton, {minimumHeight: 38});
     }
 
     let urlToSave = Pocket._urlToSave;
     let titleToSave = Pocket._titleToSave;
     Pocket._urlToSave = null;
     Pocket._titleToSave = null;
     // ViewShowing fires immediately before it creates the contents,
     // in lieu of an AfterViewShowing event, just spin the event loop.
--- a/browser/themes/shared/tabs.inc.css
+++ b/browser/themes/shared/tabs.inc.css
@@ -41,21 +41,25 @@
   margin: 0;
   padding: 0;
 }
 
 .tabbrowser-tab {
   -moz-box-align: stretch;
 }
 
-/* The selected tab should appear above adjacent tabs, .tabs-newtab-button and the highlight of #nav-bar */
+%ifndef MOZ_PHOTON_ANIMATIONS
+/* The selected tab should appear above adjacent tabs, .tabs-newtab-button and the highlight of #nav-bar.
+   This is disabled in Photon builds since we won't have overlapping tabs and we need the #nav-bar
+   to not have a lower z-index, which is causing animations to get clipped. */
 .tabbrowser-tab[visuallyselected=true] {
   position: relative;
   z-index: 2;
 }
+%endif
 
 .tab-background-middle {
   -moz-box-flex: 1;
   background-clip: padding-box;
   border-left: var(--tab-curve-half-width) solid transparent;
   border-right: var(--tab-curve-half-width) solid transparent;
   margin: 0 calc(-1 * var(--tab-curve-half-width));
 }
--- a/toolkit/modules/BrowserUtils.jsm
+++ b/toolkit/modules/BrowserUtils.jsm
@@ -373,16 +373,18 @@ this.BrowserUtils = {
    * -moz-pack-align:stretch, and thus a height which is dependant on
    * the font-size.
    *
    * @param element An element within the toolbar whose height is desired.
    * @param options An object with the following properties:
               {
                 forceLayoutFlushIfNeeded:
                   Set to true if a sync layout flush is acceptable.
+                minimumHeight:
+                  Number of pixels for the minimum value of the property.
               }
    */
   setToolbarButtonHeightProperty(element, options) {
     let window = element.ownerGlobal;
     let dwu = window.getInterface(Ci.nsIDOMWindowUtils);
     let toolbarItem = element;
     let urlBarContainer = element.closest("#urlbar-container");
     if (urlBarContainer) {
@@ -393,17 +395,19 @@ this.BrowserUtils = {
     if (!toolbarItem) {
       return;
     }
     let bounds = dwu.getBoundsWithoutFlushing(toolbarItem);
     if (!bounds.height && options.forceLayoutFlushIfNeeded) {
       bounds = toolbarItem.getBoundingClientRect();
     }
     if (bounds.height) {
-      toolbarItem.style.setProperty("--toolbarbutton-height", bounds.height + "px");
+      let minimumHeight = (options && options.minimumHeight) || 0;
+      let height = Math.max(bounds.height, minimumHeight);
+      toolbarItem.style.setProperty("--toolbarbutton-height", height + "px");
     }
   },
 
   /**
    * Track whether a toolbar is visible for a given a docShell.
    *
    * @param  {nsIDocShell} docShell  The docShell instance that a toolbar should
    *                                 be interacting with