Bug 1552141 - The address bar is filtering bookmarklets and not executing them. r=dao

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

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

browser/components/urlbar/UrlbarInput.jsm

@@ -500,6 +500,12 @@ class UrlbarInput {
     openParams.postData = postData;
 
     switch (result.type) {
+      case UrlbarUtils.RESULT_TYPE.KEYWORD: {
+        // If this result comes from a bookmark keyword, let it inherit the
+        // current document's principal, otherwise bookmarklets would break.
+        openParams.allowInheritPrincipal = true;
+        break;
+      }
       case UrlbarUtils.RESULT_TYPE.TAB_SWITCH: {
         if (this.hasAttribute("actionoverride")) {
           where = "current";

browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm

@@ -286,7 +286,14 @@ function makeUrlbarResult(tokens, info) {
         );
       case "keyword": {
         let title = info.comment;
-        if (tokens && tokens.length > 1) {
+        if (!title) {
+          // If the url doesn't have an host (e.g. javascript urls), comment
+          // will be empty, and we can't build the usual title. Thus use the url.
+          title = Services.textToSubURI.unEscapeURIForUI(
+            "UTF-8",
+            action.params.url
+          );
+        } else if (tokens && tokens.length > 1) {
           title = bundle.formatStringFromName("bookmarkKeywordSearch", [
             info.comment,
             tokens

browser/components/urlbar/UrlbarProvidersManager.jsm

@@ -357,6 +357,7 @@ class Query {
     // Filter out javascript results for safety. The provider is supposed to do
     // it, but we don't want to risk leaking these out.
     if (
+      match.type != UrlbarUtils.RESULT_TYPE.KEYWORD &&
       match.payload.url &&
       match.payload.url.startsWith("javascript:") &&
       !this.context.searchString.startsWith("javascript:") &&

browser/components/urlbar/tests/browser/browser_keywordBookmarklets.js

@@ -3,54 +3,110 @@
 
 "use strict";
 
-add_task(async function test_keyword_bookmarklet() {
-  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+add_task(async function setup() {
   let bm = await PlacesUtils.bookmarks.insert({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     title: "bookmarklet",
-    url: "javascript:'1';",
+    url: "javascript:'%s'%20",
   });
-
+  await PlacesUtils.keywords.insert({ keyword: "bm", url: bm.url });
   registerCleanupFunction(async function() {
-    BrowserTestUtils.removeTab(tab);
     await PlacesUtils.bookmarks.remove(bm);
   });
 
-  let originalPrincipal = gBrowser.contentPrincipal;
-  let originalPrincipalURI = await getPrincipalURI(tab.linkedBrowser);
-
-  await PlacesUtils.keywords.insert({ keyword: "bm", url: "javascript:'1';" });
-
-  // Enter bookmarklet keyword in the URL bar
-  gURLBar.value = "bm";
-  gURLBar.focus();
-  EventUtils.synthesizeKey("KEY_Enter");
-
-  await BrowserTestUtils.waitForContentEvent(
-    gBrowser.selectedBrowser,
-    "pageshow"
-  );
-
-  let newPrincipalURI = await getPrincipalURI(tab.linkedBrowser);
-  is(newPrincipalURI, originalPrincipalURI, "content has the same principal");
-
-  // In e10s, null principals don't round-trip so the same null principal sent
-  // from the child will be a new null principal. Verify that this is the
-  // case.
-  if (tab.linkedBrowser.isRemoteBrowser) {
-    ok(
-      originalPrincipal.isNullPrincipal &&
-        gBrowser.contentPrincipal.isNullPrincipal,
-      "both principals should be null principals in the parent"
-    );
-  } else {
-    ok(
-      gBrowser.contentPrincipal.equals(originalPrincipal),
-      "javascript bookmarklet should inherit principal"
-    );
+  let testFns = [
+    function() {
+      info("Type keyword and immediately press enter");
+      gURLBar.value = "bm";
+      gURLBar.focus();
+      EventUtils.synthesizeKey("KEY_Enter");
+      return "javascript:''%20";
+    },
+    function() {
+      info("Type keyword with searchstring and immediately press enter");
+      gURLBar.value = "bm a";
+      gURLBar.focus();
+      EventUtils.synthesizeKey("KEY_Enter");
+      return "javascript:'a'%20";
+    },
+    async function() {
+      info("Search keyword, then press enter");
+      await promiseAutocompleteResultPopup("bm");
+      let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+      Assert.equal(result.title, "javascript:'' ", "Check title");
+      EventUtils.synthesizeKey("KEY_Enter");
+      return "javascript:''%20";
+    },
+    async function() {
+      info("Search keyword with searchstring, then press enter");
+      await promiseAutocompleteResultPopup("bm a");
+      let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+      Assert.equal(result.title, "javascript:'a' ", "Check title");
+      EventUtils.synthesizeKey("KEY_Enter");
+      return "javascript:'a'%20";
+    },
+    async function() {
+      await promiseAutocompleteResultPopup("bm");
+      let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+      Assert.equal(result.title, "javascript:'' ", "Check title");
+      let element = UrlbarTestUtils.getSelectedElement(window);
+      EventUtils.synthesizeMouseAtCenter(element, {});
+      return "javascript:''%20";
+    },
+    async function() {
+      info("Search keyword with searchstring, then click");
+      await promiseAutocompleteResultPopup("bm a");
+      let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
+      Assert.equal(result.title, "javascript:'a' ", "Check title");
+      let element = UrlbarTestUtils.getSelectedElement(window);
+      EventUtils.synthesizeMouseAtCenter(element, {});
+      return "javascript:'a'%20";
+    },
+  ];
+  for (let testFn of testFns) {
+    await do_test(testFn);
   }
 });
 
+async function do_test(loadFn) {
+  await BrowserTestUtils.withNewTab(
+    {
+      gBrowser,
+    },
+    async browser => {
+      let originalPrincipal = gBrowser.contentPrincipal;
+      let originalPrincipalURI = await getPrincipalURI(browser);
+
+      let expectedUrl = await loadFn();
+      await BrowserTestUtils.waitForContentEvent(browser, "pageshow");
+      Assert.equal(gBrowser.currentURI.spec, expectedUrl);
+
+      let newPrincipalURI = await getPrincipalURI(browser);
+      Assert.equal(
+        newPrincipalURI,
+        originalPrincipalURI,
+        "content has the same principal"
+      );
+
+      // In e10s, null principals don't round-trip so the same null principal sent
+      // from the child will be a new null principal. Verify that this is the
+      // case.
+      if (browser.isRemoteBrowser) {
+        Assert.ok(
+          originalPrincipal.isNullPrincipal &&
+            gBrowser.contentPrincipal.isNullPrincipal,
+          "both principals should be null principals in the parent"
+        );
+      } else {
+        Assert.ok(
+          gBrowser.contentPrincipal.equals(originalPrincipal),
+          "javascript bookmarklet should inherit principal"
+        );
+      }
+    }
+  );
+}
+
 function getPrincipalURI(browser) {
   return ContentTask.spawn(browser, null, function() {
     return content.document.nodePrincipal.URI.spec;

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

@@ -59,7 +59,6 @@ skip-if = os != "mac" # Mac only feature
 [../browser/browser_ime_composition.js]
 [../browser/browser_inputHistory.js]
 [../browser/browser_inputHistory_emptystring.js]
-[../browser/browser_keywordBookmarklets.js]
 [../browser/browser_keepStateAcrossTabSwitches.js]
 [../browser/browser_keyword_override.js]
 [../browser/browser_keywordSearch.js]