Bug 1410629 - Clear user-entered search term already after location change. r=sebastian
authorJan Henning <jh+bugzilla@buttercookie.de>
Mon, 30 Oct 2017 14:05:32 +0100
changeset 389079 79be7ec660d58aed77af5484dd4a8dec758c496f
parent 389078 68a8bde575ef1669a3c05e8c91b009c20948c27f
child 389080 b993bccfeebb814ff8c7e79d5cfb9ffb7529f4b1
push id32777
push userarchaeopteryx@coole-files.de
push dateMon, 30 Oct 2017 22:44:45 +0000
treeherdermozilla-central@dd0f265a1300 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1410629
milestone58.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 1410629 - Clear user-entered search term already after location change. r=sebastian This is to avoid a situation where after searching and then navigating to a different URL, the user can already see the new URL (after location change), but not yet edit it (currently, the search term is only dropped after pageshow). Because location change is too early for checking the documentURI for the presence of an error page in case the load failed, we switch to checking the request's status instead. We still have to explicitly check for "about:neterror" as well, though, since the way our intent handling code displays an error page in case of an unknown protocol technically counts as a successful pageload. MozReview-Commit-ID: 8e6WQlD0sf3
mobile/android/chrome/content/browser.js
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -4119,17 +4119,16 @@ Tab.prototype = {
         GlobalEventDispatcher.sendRequest({
           type: "Content:DOMContentLoaded",
           tabID: this.id,
           bgColor: backgroundColor,
           errorType: errorType,
           metadata: this.metatags,
         });
 
-        // Reset isSearch so that the userRequested term will be erased on next page load
         this.metatags = null;
 
         if (docURI.startsWith("about:certerror") || docURI.startsWith("about:blocked")) {
           this.browser.addEventListener("click", ErrorPageEventHandler, true);
           let listener = () => {
             this.browser.removeEventListener("click", ErrorPageEventHandler, true);
             this.browser.removeEventListener("pagehide", listener, true);
           };
@@ -4313,32 +4312,23 @@ Tab.prototype = {
         break;
       }
 
       case "pageshow": {
         // The rest of this only handles pageshow for the top-level document.
         if (aEvent.originalTarget.defaultView != this.browser.contentWindow)
           return;
 
-        let target = aEvent.originalTarget;
-        let docURI = target.documentURI;
-        if (!docURI.startsWith("about:neterror") && !this.isSearch) {
-          // If this wasn't an error page and the user isn't search, don't retain the typed entry
-          this.userRequested = "";
-        }
-
         GlobalEventDispatcher.sendRequest({
           type: "Content:PageShow",
           tabID: this.id,
           userRequested: this.userRequested,
           fromCache: Tabs.useCache
         });
 
-        this.isSearch = false;
-
         if (!aEvent.persisted && Services.prefs.getBoolPref("browser.ui.linkify.phone")) {
           if (!this._linkifier)
             this._linkifier = new Linkifier();
           this._linkifier.linkifyNumbers(this.browser.contentWindow.document);
         }
 
         // Update page actions for helper apps.
         let uri = this.browser.currentURI;
@@ -4497,16 +4487,23 @@ Tab.prototype = {
       }
     }
 
     // Update the page actions URI for helper apps.
     if (BrowserApp.selectedTab == this) {
       ExternalApps.updatePageActionUri(fixedURI);
     }
 
+    if (Components.isSuccessCode(aRequest.status) &&
+        !fixedURI.displaySpec.startsWith("about:neterror") && !this.isSearch) {
+      // If this won't end up in an error page and the user isn't searching,
+      // don't retain the typed entry.
+      this.userRequested = "";
+    }
+
     let message = {
       type: "Content:LocationChange",
       tabID: this.id,
       uri: truncate(fixedURI.displaySpec, MAX_URI_LENGTH),
       userRequested: this.userRequested || "",
       baseDomain: baseDomain,
       contentType: (contentType ? contentType : ""),
       sameDocument: sameDocument,
@@ -4514,16 +4511,19 @@ Tab.prototype = {
       canGoBack: webNav.canGoBack,
       canGoForward: webNav.canGoForward,
     };
 
     GlobalEventDispatcher.sendRequest(message);
 
     notifyManifestStatus(this);
 
+    // Reset isSearch so that the userRequested term will be erased on next location change.
+    this.isSearch = false;
+
     if (!sameDocument) {
       // XXX This code assumes that this is the earliest hook we have at which
       // browser.contentDocument is changed to the new document we're loading
       this.contentDocumentIsDisplayed = false;
       this.hasTouchListener = false;
       Services.obs.notifyObservers(this.browser, "Session:NotifyLocationChange");
     }
   },