Bug 1365780 - Consistently dispatch the TabAttrModified event and update the title bar when setting tab labels. r?mikedeboer draft
authorDão Gottwald <dao@mozilla.com>
Fri, 19 May 2017 14:19:06 +0200
changeset 581171 32be8d97472f5774964455edc78f6feb73a5c780
parent 580912 8e98dab5054dd093a37ba20c62cf0523e484cfbd
child 629511 aa41255f0fa8e9fc2588052cfa20ef3906bb7e8d
push id59798
push userdgottwald@mozilla.com
push dateFri, 19 May 2017 12:21:48 +0000
reviewersmikedeboer
bugs1365780
milestone55.0a1
Bug 1365780 - Consistently dispatch the TabAttrModified event and update the title bar when setting tab labels. r?mikedeboer MozReview-Commit-ID: 1SNHtyw5hQ8
browser/base/content/tabbrowser.xml
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/test/browser_tab_label_during_restore.js
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -995,21 +995,25 @@
 
       <method name="getWindowTitleForBrowser">
         <parameter name="aBrowser"/>
         <body>
           <![CDATA[
             var newTitle = "";
             var docElement = this.ownerDocument.documentElement;
             var sep = docElement.getAttribute("titlemenuseparator");
-
-            // Strip out any null bytes in the content title, since the
-            // underlying widget implementations of nsWindow::SetTitle pass
-            // null-terminated strings to system APIs.
-            var docTitle = aBrowser.contentTitle.replace(/\0/g, "");
+            let tab = this.getTabForBrowser(aBrowser);
+            let docTitle;
+
+            if (tab._labelIsContentTitle) {
+              // Strip out any null bytes in the content title, since the
+              // underlying widget implementations of nsWindow::SetTitle pass
+              // null-terminated strings to system APIs.
+              docTitle = tab.getAttribute("label").replace(/\0/g, "");
+            }
 
             if (!docTitle)
               docTitle = docElement.getAttribute("titledefault");
 
             var modifier = docElement.getAttribute("titlemodifier");
             if (docTitle) {
               newTitle += docElement.getAttribute("titlepreface");
               newTitle += docTitle;
@@ -1407,23 +1411,24 @@
       <method name="setTabTitleLoading">
         <parameter name="aTab"/>
         <body/>
       </method>
 
       <method name="setInitialTabTitle">
         <parameter name="aTab"/>
         <parameter name="aTitle"/>
+        <parameter name="aOptions"/>
         <body><![CDATA[
           if (aTitle) {
-            aTab.setAttribute("label", aTitle);
-
             // Don't replace the set label with the empty tab label or the URL
             // while the tab is loading.
             aTab._suppressTransientPlaceholderLabel = true;
+
+            this._setTabLabel(aTab, aTitle, aOptions);
           }
         ]]></body>
       </method>
 
       <method name="setTabTitle">
         <parameter name="aTab"/>
         <body>
           <![CDATA[
@@ -1432,17 +1437,20 @@
 
             if (aTab._suppressTransientPlaceholderLabel) {
               if (!title) {
                 return false;
               }
               delete aTab._suppressTransientPlaceholderLabel;
             }
 
-            if (!title) {
+            let isContentTitle = false;
+            if (title) {
+              isContentTitle = true;
+            } else {
               if (browser.currentURI.spec) {
                 try {
                   title = this.mURIFixup.createExposableURI(browser.currentURI).spec;
                 } catch (ex) {
                   title = browser.currentURI.spec;
                 }
               }
 
@@ -1460,24 +1468,43 @@
                 let brandBundle = document.getElementById("bundle_brand");
                 let brandShortName = brandBundle.getString("brandShortName");
                 title = gNavigatorBundle.getFormattedString("customizeMode.tabTitle",
                                                             [ brandShortName ]);
               } else // Still no title?  Fall back to our untitled string.
                 title = this.mStringBundle.getString("tabs.emptyTabTitle");
             }
 
-            if (aTab.label == title)
+            return this._setTabLabel(aTab, title, { isContentTitle });
+          ]]>
+        </body>
+      </method>
+
+      <method name="_setTabLabel">
+        <parameter name="aTab"/>
+        <parameter name="aLabel"/>
+        <parameter name="aOptions"/>
+        <body>
+          <![CDATA[
+            if (!aLabel || aTab.getAttribute("label") == aLabel) {
               return false;
-
-            aTab.label = title;
-            this._tabAttrModified(aTab, ["label"]);
-
-            if (aTab.selected)
+            }
+
+            aTab.setAttribute("label", aLabel);
+            aTab._labelIsContentTitle = aOptions && aOptions.isContentTitle;
+
+            // Dispatch TabAttrModified event unless we're setting the label
+            // before the TabOpen event was dispatched.
+            if (!aOptions || !aOptions.beforeTabOpen) {
+              this._tabAttrModified(aTab, ["label"]);
+            }
+
+            if (aTab.selected) {
               this.updateTitlebar();
+            }
 
             return true;
           ]]>
         </body>
       </method>
 
       <method name="loadOneTab">
         <parameter name="aURI"/>
@@ -2308,17 +2335,17 @@
 
             var uriIsAboutBlank = aURI == "about:blank";
 
             if (!aNoInitialLabel) {
               if (isBlankPageURL(aURI)) {
                 t.setAttribute("label", this.mStringBundle.getString("tabs.emptyTabTitle"));
               } else {
                 // Set URL as label so that the tab isn't empty initially.
-                this.setInitialTabTitle(t, aURI);
+                this.setInitialTabTitle(t, aURI, { beforeTabOpen: true });
               }
             }
 
             if (aIsPrerendered) {
               t.setAttribute("hidden", "true");
             }
 
             // Related tab inherits current tab's user context unless a different
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -2641,17 +2641,17 @@ var SessionStoreInternal = {
       }
     }
 
     let activePageData = tabData.entries[tabData.index - 1] || null;
 
     // If the page has a title, set it.
     if (activePageData) {
       if (activePageData.title) {
-        win.gBrowser.setInitialTabTitle(tab, activePageData.title);
+        win.gBrowser.setInitialTabTitle(tab, activePageData.title, { isContentTitle: true });
       } else if (activePageData.url != "about:blank") {
         win.gBrowser.setInitialTabTitle(tab, activePageData.url);
       }
     }
 
     // Restore the tab icon.
     if ("image" in tabData) {
       // Use the serialized contentPrincipal with the new icon load.
--- a/browser/components/sessionstore/test/browser_tab_label_during_restore.js
+++ b/browser/components/sessionstore/test/browser_tab_label_during_restore.js
@@ -46,40 +46,44 @@ add_task(async function() {
 
   await browserLoadedPromise;
   const CONTENT_TITLE = firstTab.linkedBrowser.contentTitle;
   is(firstTab.linkedBrowser.currentURI.spec, TEST_URL, "correct URL loaded in first tab");
   is(typeof CONTENT_TITLE, "string", "content title is a string");
   isnot(CONTENT_TITLE.length, 0, "content title isn't empty");
   isnot(CONTENT_TITLE, TEST_URL, "content title is different from the URL");
   is(firstTab.label, CONTENT_TITLE, "first tab displays content title");
+  ok(document.title.startsWith(CONTENT_TITLE), "title bar displays content title");
   ok(secondTab.hasAttribute("pending"), "second tab is pending");
   is(secondTab.label, TEST_URL, "second tab displays URL as its title");
 
   info("selecting the second tab");
   let checkLabelChangeCount = observeLabelChanges(secondTab);
   browserLoadedPromise = BrowserTestUtils.browserLoaded(secondTab.linkedBrowser, false, TEST_URL);
   gBrowser.selectedTab = secondTab;
   await browserLoadedPromise;
   ok(!secondTab.hasAttribute("pending"), "second tab isn't pending anymore");
-  is(gBrowser.selectedTab.label, CONTENT_TITLE, "second tab displays content title");
+  is(secondTab.label, CONTENT_TITLE, "second tab displays content title");
+  ok(document.title.startsWith(CONTENT_TITLE), "title bar displays content title");
   checkLabelChangeCount(1);
 
   info("restoring the modified browser state");
   await TabStateFlusher.flushWindow(window);
   await promiseBrowserState(SessionStore.getBrowserState());
   [firstTab, secondTab] = gBrowser.tabs;
   is(secondTab, gBrowser.selectedTab, "second tab is selected after restoring");
+  ok(document.title.startsWith(CONTENT_TITLE), "title bar displays content title");
   ok(firstTab.hasAttribute("pending"), "first tab is pending after restoring");
   is(firstTab.label, CONTENT_TITLE, "first tab displays content title in pending state");
 
   info("selecting the first tab");
   checkLabelChangeCount = observeLabelChanges(firstTab);
   let tabContentRestored = TestUtils.topicObserved("sessionstore-debug-tab-restored");
   gBrowser.selectedTab = firstTab;
+  ok(document.title.startsWith(CONTENT_TITLE), "title bar displays content title");
   await tabContentRestored;
   ok(!firstTab.hasAttribute("pending"), "first tab isn't pending anymore");
   checkLabelChangeCount(0);
   is(firstTab.label, CONTENT_TITLE, "first tab displays content title after restoring content");
 
   await promiseBrowserState(BACKUP_STATE);
 });