Bug 1394188 - Make tab labels not change during reload. r=bzbarsky,florian
authorDão Gottwald <dao@mozilla.com>
Mon, 17 Dec 2018 12:33:29 +0000
changeset 450954 8dd7fed5281823eb2ff87d9a56fc0a6f1e97e494
parent 450953 b643a8a7b3280ca4f719dc0e9479ec25055d2109
child 450955 b7d06736403e5f07f8d1b68ea7345f6486eea4eb
push id35222
push useraiakab@mozilla.com
push dateMon, 17 Dec 2018 22:01:00 +0000
treeherdermozilla-central@edf1f05e9d00 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky, florian
bugs1394188
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1394188 - Make tab labels not change during reload. r=bzbarsky,florian Differential Revision: https://phabricator.services.mozilla.com/D14602
browser/base/content/tabbrowser.js
browser/base/content/test/tabs/browser.ini
browser/base/content/test/tabs/browser_tab_label_during_reload.js
docshell/base/nsDocShell.cpp
uriloader/base/nsIWebProgressListener.idl
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -5079,41 +5079,42 @@ class TabProgressListener {
 
   onLocationChange(aWebProgress, aRequest, aLocation, aFlags) {
     // OnLocationChange is called for both the top-level content
     // and the subframes.
     let topLevel = aWebProgress.isTopLevel;
 
     if (topLevel) {
       let isSameDocument = !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
+      let isReload = !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_RELOAD);
+      let isErrorPage = !!(aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE);
+
       // We need to clear the typed value
       // if the document failed to load, to make sure the urlbar reflects the
       // failed URI (particularly for SSL errors). However, don't clear the value
       // if the error page's URI is about:blank, because that causes complete
       // loss of urlbar contents for invalid URI errors (see bug 867957).
       // Another reason to clear the userTypedValue is if this was an anchor
       // navigation initiated by the user.
       // Finally, we do insert the URL if this is a same-document navigation
       // and the user cleared the URL manually.
       if (this.mBrowser.didStartLoadSinceLastUserTyping() ||
-          ((aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) &&
-            aLocation.spec != "about:blank") ||
+          (isErrorPage && aLocation.spec != "about:blank") ||
           (isSameDocument && this.mBrowser.inLoadURI) ||
           (isSameDocument && !this.mBrowser.userTypedValue)) {
         this.mBrowser.userTypedValue = null;
       }
 
       // If the tab has been set to "busy" outside the stateChange
       // handler below (e.g. by sessionStore.navigateAndRestore), and
       // the load results in an error page, it's possible that there
       // isn't any (STATE_IS_NETWORK & STATE_STOP) state to cause busy
       // attribute being removed. In this case we should remove the
       // attribute here.
-      if ((aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) &&
-          this.mTab.hasAttribute("busy")) {
+      if (isErrorPage && this.mTab.hasAttribute("busy")) {
         this.mTab.removeAttribute("busy");
         gBrowser._tabAttrModified(this.mTab, ["busy"]);
       }
 
       // If the browser was playing audio, we should remove the playing state.
       if (this.mTab.hasAttribute("soundplaying") && !isSameDocument) {
         clearTimeout(this.mTab._soundPlayingAttrRemovalTimer);
         this.mTab._soundPlayingAttrRemovalTimer = 0;
@@ -5130,17 +5131,19 @@ class TabProgressListener {
         let findBar = gBrowser.getCachedFindBar(this.mTab);
 
         // Close the Find toolbar if we're in old-style TAF mode
         if (findBar.findMode != findBar.FIND_NORMAL) {
           findBar.close();
         }
       }
 
-      gBrowser.setTabTitle(this.mTab);
+      if (!isReload) {
+        gBrowser.setTabTitle(this.mTab);
+      }
 
       // Don't clear the favicon if this tab is in the pending
       // state, as SessionStore will have set the icon for us even
       // though we're pointed at an about:blank. Also don't clear it
       // if onLocationChange was triggered by a pushState or a
       // replaceState (bug 550565) or a hash change (bug 408415).
       if (!this.mTab.hasAttribute("pending") &&
           aWebProgress.isLoadingDocument &&
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -63,16 +63,17 @@ skip-if = !e10s # Pref and test only rel
 [browser_pinnedTabs_clickOpen.js]
 [browser_pinnedTabs_closeByKeyboard.js]
 [browser_pinnedTabs.js]
 [browser_positional_attributes.js]
 skip-if = (verify && (os == 'win' || os == 'mac'))
 [browser_preloadedBrowser_zoom.js]
 [browser_reload_deleted_file.js]
 skip-if = (debug && os == 'mac') || (debug && os == 'linux' && bits == 64) #Bug 1421183, disabled on Linux/OSX for leaked windows
+[browser_tab_label_during_reload.js]
 [browser_tabCloseProbes.js]
 [browser_tabContextMenu_keyboard.js]
 [browser_tabReorder_overflow.js]
 [browser_tabReorder.js]
 [browser_tabSpinnerProbe.js]
 skip-if = !e10s # Tab spinner is e10s only.
 [browser_tabSuccessors.js]
 [browser_tabSwitchPrintPreview.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/browser_tab_label_during_reload.js
@@ -0,0 +1,30 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function() {
+  let tab = gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser, "about:preferences");
+  let browser = tab.linkedBrowser;
+  let labelChanges = 0;
+  let attrModifiedListener = event => {
+    if (event.detail.changed.includes("label")) {
+      labelChanges++;
+    }
+  };
+  tab.addEventListener("TabAttrModified", attrModifiedListener);
+
+  await BrowserTestUtils.browserLoaded(browser);
+  is(labelChanges, 1, "number of label changes during initial load");
+  isnot(tab.label, "", "about:preferences tab label isn't empty");
+  isnot(tab.label, "about:preferences", "about:preferences tab label isn't the URI");
+  is(tab.label, browser.contentTitle, "about:preferences tab label matches browser.contentTitle");
+
+  labelChanges = 0;
+  browser.reload();
+  await BrowserTestUtils.browserLoaded(browser);
+  is(labelChanges, 0, "number of label changes during reload");
+
+  tab.removeEventListener("TabAttrModified", attrModifiedListener);
+  gBrowser.removeTab(tab);
+});
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -8305,17 +8305,19 @@ nsresult nsDocShell::CreateContentViewer
     // native messages might be starved.
     FavorPerformanceHint(true);
   }
 
   if (errorOnLocationChangeNeeded) {
     FireOnLocationChange(this, failedChannel, failedURI,
                          LOCATION_CHANGE_ERROR_PAGE);
   } else if (onLocationChangeNeeded) {
-    FireOnLocationChange(this, aRequest, mCurrentURI, 0);
+    uint32_t locationFlags =
+        (mLoadType & LOAD_CMD_RELOAD) ? uint32_t(LOCATION_CHANGE_RELOAD) : 0;
+    FireOnLocationChange(this, aRequest, mCurrentURI, locationFlags);
   }
 
   return NS_OK;
 }
 
 nsresult nsDocShell::NewContentViewerObj(const nsACString& aContentType,
                                          nsIRequest* aRequest,
                                          nsILoadGroup* aLoadGroup,
--- a/uriloader/base/nsIWebProgressListener.idl
+++ b/uriloader/base/nsIWebProgressListener.idl
@@ -407,19 +407,24 @@ interface nsIWebProgressListener : nsISu
    *   Generally speaking, |aURI| and |aRequest| are the original data. DOM
    *   |window.location.href| is also the original location, while
    *   |document.documentURI| is the redirected location. Sometimes |aURI| is
    *   <about:blank> and |aRequest| is null when the original data does not
    +   remain.
    *
    *   |aWebProgress| does NOT set this flag when it did not try to load a new
    *   document. In this case, it should set LOCATION_CHANGE_SAME_DOCUMENT.
+   *
+   * LOCATION_CHANGE_RELOAD
+   *   This flag is on when reloading the current page, either from
+   *   location.reload() or the browser UI.
    */
   const unsigned long LOCATION_CHANGE_SAME_DOCUMENT   = 0x00000001;
   const unsigned long LOCATION_CHANGE_ERROR_PAGE      = 0x00000002;
+  const unsigned long LOCATION_CHANGE_RELOAD          = 0x00000004;
 
   /**
    * Called when the location of the window being watched changes.  This is not
    * when a load is requested, but rather once it is verified that the load is
    * going to occur in the given window.  For instance, a load that starts in a
    * window might send progress and status messages for the new site, but it
    * will not send the onLocationChange until we are sure that we are loading
    * this new page here.