Bug 1473514: Display an empty space for the tab icon while waiting for the real favicon to appear. r=dao
authorDave Townsend <dtownsend@oxymoronical.com>
Wed, 29 Aug 2018 18:27:36 +0000
changeset 482295 141d116489a7de2cd69ca7bd5eb494d1a1dfafbc
parent 482294 cf15e6faf98319bbab89a59fd2a72527a3b6fd29
child 482296 c21566b512f1a7339bb3d0e286865dadc5e25ace
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersdao
bugs1473514
milestone63.0a1
Bug 1473514: Display an empty space for the tab icon while waiting for the real favicon to appear. r=dao This adds a simple empty box that is displayed when we're still loading an icon but are no longer showing the throbber. Ideally I'd like to keep showing the throbber and maintain the busy state but that seems more risky for now. Differential Revision: https://phabricator.services.mozilla.com/D2364
browser/base/content/browser.css
browser/base/content/browser.js
browser/base/content/tabbrowser.css
browser/base/content/tabbrowser.xml
browser/base/content/test/favicons/browser.ini
browser/base/content/test/favicons/browser_title_flicker.js
browser/base/content/test/favicons/file_with_slow_favicon.html
browser/modules/FaviconLoader.jsm
browser/themes/shared/tabs.inc.css
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -180,16 +180,17 @@ panelview[mainview] > .panel-header {
 .tab-icon-image[fadein],
 .tab-close-button[fadein],
 .tabbrowser-tab[fadein]::after,
 .tab-background[fadein] {
   /* This transition is only wanted for opening tabs. */
   transition: visibility 0ms 25ms;
 }
 
+.tab-icon-pending:not([fadein]),
 .tab-icon-image:not([fadein]),
 .tab-close-button:not([fadein]),
 .tabbrowser-tab:not([fadein])::after,
 .tab-background:not([fadein]) {
   visibility: hidden;
 }
 
 .tab-label:not([fadein]),
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -3618,62 +3618,87 @@ var newWindowButtonObserver = {
       }
     }
   }
 };
 const DOMEventHandler = {
   init() {
     let mm = window.messageManager;
     mm.addMessageListener("Link:AddFeed", this);
+    mm.addMessageListener("Link:LoadingIcon", this);
     mm.addMessageListener("Link:SetIcon", this);
+    mm.addMessageListener("Link:SetFailedIcon", this);
     mm.addMessageListener("Link:AddSearch", this);
     mm.addMessageListener("Meta:SetPageInfo", this);
   },
 
   receiveMessage(aMsg) {
     switch (aMsg.name) {
       case "Link:AddFeed":
         let link = {type: aMsg.data.type, href: aMsg.data.href, title: aMsg.data.title};
         FeedHandler.addFeed(link, aMsg.target);
         break;
 
+      case "Link:LoadingIcon":
+        if (aMsg.data.canUseForTab) {
+          this.setPendingIcon(aMsg.target);
+        }
+        break;
+
       case "Link:SetIcon":
         this.setIconFromLink(aMsg.target, aMsg.data.originalURL, aMsg.data.canUseForTab,
                              aMsg.data.expiration, aMsg.data.iconURL);
         break;
 
+      case "Link:SetFailedIcon":
+        if (aMsg.data.canUseForTab) {
+          this.clearPendingIcon(aMsg.target);
+        }
+        break;
+
       case "Link:AddSearch":
         this.addSearch(aMsg.target, aMsg.data.engine, aMsg.data.url);
         break;
 
       case "Meta:SetPageInfo":
         this.setPageInfo(aMsg.data);
         break;
     }
   },
 
   setPageInfo(aData) {
     const {url, description, previewImageURL} = aData;
     gBrowser.setPageInfo(url, description, previewImageURL);
     return true;
   },
 
+  setPendingIcon(aBrowser) {
+    let tab = gBrowser.getTabForBrowser(aBrowser);
+    tab.setAttribute("pendingicon", "true");
+  },
+
+  clearPendingIcon(aBrowser) {
+    let tab = gBrowser.getTabForBrowser(aBrowser);
+    tab.removeAttribute("pendingicon");
+  },
+
   setIconFromLink(aBrowser, aOriginalURL, aCanUseForTab, aExpiration, aIconURL) {
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
       return false;
 
     try {
       PlacesUIUtils.loadFavicon(aBrowser, Services.scriptSecurityManager.getSystemPrincipal(),
                                 makeURI(aOriginalURL), aExpiration, makeURI(aIconURL));
     } catch (ex) {
       Cu.reportError(ex);
     }
 
     if (aCanUseForTab) {
+      this.clearPendingIcon(aBrowser);
       gBrowser.setIcon(tab, aIconURL, aOriginalURL);
     }
     return true;
   },
 
   addSearch(aBrowser, aEngine, aURL) {
     let tab = gBrowser.getTabForBrowser(aBrowser);
     if (!tab)
--- a/browser/base/content/tabbrowser.css
+++ b/browser/base/content/tabbrowser.css
@@ -3,16 +3,18 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 .tabbrowser-arrowscrollbox {
   -moz-binding: url("chrome://browser/content/tabbrowser.xml#tabbrowser-arrowscrollbox");
 }
 
 .tab-close-button[pinned],
 #tabbrowser-tabs[closebuttons="activetab"] > .tabbrowser-tab > .tab-stack > .tab-content > .tab-close-button:not([selected="true"]),
+.tab-icon-pending:not([pendingicon]),
+.tab-icon-pending[busy],
 .tab-icon-image:not([src]):not([pinned]):not([crashed])[selected],
 .tab-icon-image:not([src]):not([pinned]):not([crashed]):not([sharing]),
 .tab-icon-image[busy],
 .tab-throbber:not([busy]),
 .tab-throbber-fallback:not([busy]),
 .tab-icon-sound:not([soundplaying]):not([muted]):not([activemedia-blocked]),
 .tab-icon-sound[pinned],
 .tab-sharing-icon-overlay,
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1670,16 +1670,19 @@
           <xul:hbox xbl:inherits="fadein,pinned,busy,progress,selected=visuallyselected"
                     anonid="tab-throbber"
                     class="tab-throbber"
                     layer="true"/>
           <xul:image xbl:inherits="fadein,pinned,busy,progress,selected=visuallyselected"
                      class="tab-throbber-fallback"
                      role="presentation"
                      layer="true"/>
+          <xul:hbox xbl:inherits="fadein,pinned,busy,progress,selected=visuallyselected,pendingicon"
+                    anonid="tab-icon-pending"
+                    class="tab-icon-pending"/>
           <xul:image xbl:inherits="src=image,triggeringprincipal=iconloadingprincipal,requestcontextid,fadein,pinned,selected=visuallyselected,busy,crashed,sharing"
                      anonid="tab-icon-image"
                      class="tab-icon-image"
                      validate="never"
                      role="presentation"/>
           <xul:image xbl:inherits="sharing,selected=visuallyselected,pinned"
                      anonid="sharing-icon"
                      class="tab-sharing-icon-overlay"
--- a/browser/base/content/test/favicons/browser.ini
+++ b/browser/base/content/test/favicons/browser.ini
@@ -50,8 +50,11 @@ support-files =
   file_favicon_redirect.ico^headers^
 [browser_rooticon.js]
 support-files =
   blank.html
 [browser_mixed_content.js]
 support-files =
   file_insecure_favicon.html
   file_favicon.png
+[browser_title_flicker.js]
+support-files =
+  file_with_slow_favicon.html
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/favicons/browser_title_flicker.js
@@ -0,0 +1,43 @@
+// Verify that the title doesn't flicker if the icon takes too long to load.
+// We expect to see events in the following order:
+// "label" added to tab
+// "busy" removed from tab
+// icon available
+// In all those cases the title should be in the same position.
+
+function waitForAttributeChange(tab, attr) {
+  info(`Waiting for attribute ${attr}`);
+  return new Promise(resolve => {
+    let listener = (event) => {
+      if (event.detail.changed.includes(attr)) {
+        tab.removeEventListener("TabAttrModified", listener);
+        resolve();
+      }
+    };
+
+    tab.addEventListener("TabAttrModified", listener);
+  });
+}
+
+add_task(async () => {
+  const testPath = "http://example.com/browser/browser/base/content/test/favicons/";
+
+  await BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" }, async (browser) => {
+    let tab = gBrowser.getTabForBrowser(browser);
+    BrowserTestUtils.loadURI(browser, testPath + "file_with_slow_favicon.html");
+
+    await waitForAttributeChange(tab, "label");
+    ok(tab.hasAttribute("busy"), "Should have seen the busy attribute");
+    let label = document.getAnonymousElementByAttribute(tab, "anonid", "tab-label");
+    let bounds = label.getBoundingClientRect();
+
+    await waitForAttributeChange(tab, "busy");
+    ok(!tab.hasAttribute("busy"), "Should have seen the busy attribute removed");
+    let newBounds = label.getBoundingClientRect();
+    is(bounds.x, newBounds.left, "Should have seen the title in the same place.");
+
+    await waitForFaviconMessage(true);
+    newBounds = label.getBoundingClientRect();
+    is(bounds.x, newBounds.left, "Should have seen the title in the same place.");
+  });
+});
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/favicons/file_with_slow_favicon.html
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for title flicker</title>
+</head>
+<body>
+  <!-- Putting the icon down here means we won't start loading it until the doc is fully parsed -->
+  <link rel="icon" href="file_generic_favicon.ico">
+</body>
+</html>
--- a/browser/modules/FaviconLoader.jsm
+++ b/browser/modules/FaviconLoader.jsm
@@ -380,16 +380,22 @@ class IconLoader {
         originalURL: iconInfo.iconUri.spec,
         canUseForTab: !iconInfo.isRichIcon,
         expiration: undefined,
         iconURL: iconInfo.iconUri.spec,
       });
       return;
     }
 
+    // Let the main process that a tab icon is possibly coming.
+    this.mm.sendAsyncMessage("Link:LoadingIcon", {
+      originalURL: iconInfo.iconUri.spec,
+      canUseForTab: !iconInfo.isRichIcon,
+    });
+
     try {
       this._loader = new FaviconLoad(iconInfo);
       let { dataURL, expiration } = await this._loader.load();
 
       this.mm.sendAsyncMessage("Link:SetIcon", {
         originalURL: iconInfo.iconUri.spec,
         canUseForTab: !iconInfo.isRichIcon,
         expiration,
--- a/browser/themes/shared/tabs.inc.css
+++ b/browser/themes/shared/tabs.inc.css
@@ -148,34 +148,37 @@
   100% {
     opacity: 0;
     transform: scale(40);
   }
 }
 
 .tab-throbber,
 .tab-throbber-fallback,
+.tab-icon-pending,
 .tab-icon-image,
 .tab-sharing-icon-overlay,
 .tab-icon-sound,
 .tab-close-button {
   margin-top: 1px;
 }
 
 .tab-throbber,
 .tab-throbber-fallback,
+.tab-icon-pending,
 .tab-icon-image,
 .tab-sharing-icon-overlay {
   height: 16px;
   width: 16px;
 }
 
 .tab-throbber:not([pinned]),
 .tab-throbber-fallback:not([pinned]),
 .tab-sharing-icon-overlay:not([pinned]),
+.tab-icon-pending:not([pinned]),
 .tab-icon-image:not([pinned]) {
   margin-inline-end: 6px;
 }
 
 :root[sessionrestored] .tab-throbber[busy] {
   position: relative;
   overflow: hidden;
 }