Bug 1573753 - Copy the urlbar value when pageproxystate is invalid and there's no result. r=adw

When copying from the input field, when the value has already been confirmed
by the user (either by picking a result or Enter), we cannot use the selected
result information, because it has already been cleared by _loadURL setting
the value to the final one.
If the page already finished loading, pageproxystate is valid and we can use
currentURI, but if the page takes a long time to load, or the load is canceled,
we need a fallback, because we don't want to interrupt the copy operation.

In this case, the current value can be either a valid url or a search string.
If it's a valid url, we can just go through the usual URI handling, trying to
make it nicer. After all, we'd do the same just after the page is done loading.
Otherwise, we fallback to the currently selected value.

This works also if the user is still in the process of selecting a result
and tries to copy from the input field, because on result selection we set
either a valid url or a non-url.

Differential Revision: https://phabricator.services.mozilla.com/D50832

--HG--
extra : moz-landing-system : lando

browser/components/urlbar/UrlbarInput.jsm

@@ -1233,29 +1233,18 @@ class UrlbarInput {
     if (this.getAttribute("pageproxystate") == "valid") {
       uri = this.window.gBrowser.currentURI;
     } else {
-      // We're dealing with an autocompleted value.
-      if (!this._resultForCurrentValue) {
-        throw new Error(
-          "UrlbarInput: Should have a UrlbarResult since " +
-            "pageproxystate != 'valid' and valueIsTyped == false"
-        );
-      }
-      let resultURL = this._resultForCurrentValue.payload.url;
-      if (!resultURL) {
-        return selectedVal;
-      }
-
+      // The value could be:
+      // 1. a trimmed url, set by selecting a result
+      // 2. a search string set by selecting a result
+      // 3. a url that was confirmed but didn't finish loading yet
+      // If it's an url the untrimmedValue should resolve to a valid URI,
+      // otherwise it's a search string that should be copied as-is.
       try {
-        uri = Services.uriFixup.createFixupURI(
-          resultURL,
-          Services.uriFixup.FIXUP_FLAG_NONE
-        );
-      } catch (e) {}
-      if (!uri) {
+        uri = Services.io.newURI(this._untrimmedValue);
+      } catch (ex) {
         return selectedVal;
       }
     }
-
     uri = this.makeURIReadable(uri);
 
     // If the entire URL is selected, just use the actual loaded URI,

browser/components/urlbar/tests/browser/browser.ini

@@ -118,6 +118,9 @@ support-files =
   file_blank_but_not_blank.html
 [browser_urlbar_collapseOnChromeMousedown.js]
 [browser_urlbar_content_opener.js]
+[browser_urlbar_copy_during_load.js]
+support-files =
+  slow-page.sjs
 [browser_urlbar_display_selectedAction_Extensions.js]
 [browser_urlbar_empty_search.js]
 [browser_urlbar_event_telemetry.js]

browser/components/urlbar/tests/browser/browser_urlbar_copy_during_load.js

@@ -0,0 +1,51 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that copying from the urlbar page works correctly after a result is
+// confirmed but takes a while to load.
+
+add_task(async function() {
+  const SLOW_PAGE =
+    getRootDirectory(gTestPath).replace(
+      "chrome://mochitests/content",
+      "http://www.example.com"
+    ) + "slow-page.sjs";
+
+  await BrowserTestUtils.withNewTab(gBrowser, async tab => {
+    gURLBar.focus();
+    gURLBar.value = SLOW_PAGE;
+    let promise = TestUtils.waitForCondition(
+      () => gURLBar.getAttribute("pageproxystate") == "invalid"
+    );
+    EventUtils.synthesizeKey("KEY_Enter");
+    info("wait for the initial conditions");
+    await promise;
+
+    info("Copy the whole url");
+    await SimpleTest.promiseClipboardChange(SLOW_PAGE, () => {
+      gURLBar.select();
+      goDoCommand("cmd_copy");
+    });
+
+    info("Copy the initial part of the url, as a different valid url");
+    await SimpleTest.promiseClipboardChange(
+      SLOW_PAGE.substring(0, SLOW_PAGE.indexOf("slow-page.sjs")),
+      () => {
+        gURLBar.selectionStart = 0;
+        gURLBar.selectionEnd = gURLBar.value.indexOf("slow-page.sjs");
+        goDoCommand("cmd_copy");
+      }
+    );
+
+    // This is apparently necessary to avoid a timeout on mochitest shutdown(!?)
+    let browserStoppedPromise = BrowserTestUtils.browserStopped(
+      gBrowser,
+      null,
+      true
+    );
+    BrowserStop();
+    await browserStoppedPromise;
+  });
+});

browser/components/urlbar/tests/browser/slow-page.sjs

@@ -11,9 +11,13 @@ function handleRequest(request, response) {
   }
   response.processAsync();
   timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-  timer.init(() => {
-    response.setHeader("Content-Type", "text/html", false);
-    response.write("<body>This was the slow load. You should never see this.</body>");
-    response.finish();
-  }, DELAY_MS, Ci.nsITimer.TYPE_ONE_SHOT);
+  timer.init(
+    () => {
+      response.setHeader("Content-Type", "text/html", false);
+      response.write("<body>This is a slow loading page.</body>");
+      response.finish();
+    },
+    DELAY_MS,
+    Ci.nsITimer.TYPE_ONE_SHOT
+  );
 }