Bug 1065265 - Improve infrastructure dealing with moz-action: autocomplete results. r=mak

--HG-- extra : transplant_source : %C9%C8x%E6%A3%E5%89%7F%B05%F2%82%FE%05%EF%E7%F5%0E%C0.
2014-09-11 22:50:08 +12:00 · 2014-09-11 22:50:08 +12:00 · ee1474e7da
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@ -393,7 +393,8 @@ panel[noactions] > richlistbox > richlistitem[type~="action"] > .ac-url-box > .a
  visibility: visible;
 }

-#urlbar:not([actiontype]) > #urlbar-display-box {
+#urlbar:not([actiontype]) > #urlbar-display-box,
+#urlbar:not([actiontype="switchtab"]) > #urlbar-display-box > .urlbar-display-switchtab {
  display: none;
 }

--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@ -756,7 +756,7 @@
                </hbox>
              </box>
              <box id="urlbar-display-box" align="center">
-                <label id="urlbar-display" value="&urlbar.switchToTab.label;"/>
+                <label class="urlbar-display urlbar-display-switchtab" value="&urlbar.switchToTab.label;"/>
              </box>
              <hbox id="urlbar-icons">
                <image id="page-report-button"
--- a/browser/base/content/test/general/browser_bug555767.js
+++ b/browser/base/content/test/general/browser_bug555767.js
@ -34,7 +34,7 @@ function test() {
       "The test tab doesn't have the busy attribute");

    // Set the urlbar to include the moz-action
-    gURLBar.value = "moz-action:switchtab," + testURL;
+    gURLBar.value = "moz-action:switchtab," + JSON.stringify({url: testURL});
    // Focus the urlbar so we can press enter
    gURLBar.focus();

--- a/browser/base/content/test/general/browser_bug556061.js
+++ b/browser/base/content/test/general/browser_bug556061.js
@ -3,7 +3,7 @@
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

 let testURL = "http://example.org/browser/browser/base/content/test/general/dummy_page.html";
-let testActionURL = "moz-action:switchtab," + testURL;
+let testActionURL = "moz-action:switchtab," + JSON.stringify({url: testURL});
 testURL = gURLBar.trimValue(testURL);
 let testTab;

--- a/browser/base/content/test/general/browser_bug816527.js
+++ b/browser/base/content/test/general/browser_bug816527.js
@ -72,7 +72,7 @@ function test() {
           "The test tab doesn't have the busy attribute");

        // Set the urlbar to include the moz-action
-        aDestWindow.gURLBar.value = "moz-action:switchtab," + testURL;
+        aDestWindow.gURLBar.value = "moz-action:switchtab," + JSON.stringify({url: testURL});
        // Focus the urlbar so we can press enter
        aDestWindow.gURLBar.focus();

--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@ -150,7 +150,12 @@
          var action = this._parseActionUrl(aValue);

          if (action) {
-            returnValue = action.param;
+            switch (action.type) {
+              case "switchtab": {
+                returnValue = action.params.url;
+                break;
+              }
+            }
          }

          // Set the actiontype only if the user is not overriding actions.
@ -279,9 +284,9 @@

          let matchLastLocationChange = true;
          if (action) {
-            url = action.param;
            if (this.hasAttribute("actiontype")) {
              if (action.type == "switchtab") {
+                url = action.params.url;
                this.handleRevert();
                let prevTab = gBrowser.selectedTab;
                if (switchToTabHavingURI(url) &&
@ -717,9 +722,28 @@
          if (!aUrl.startsWith("moz-action:"))
            return null;

-          // url is in the format moz-action:ACTION,PARAM
-          let [, action, param] = aUrl.match(/^moz-action:([^,]+),(.*)$/);
-          return {type: action, param: param};
+          // URL is in the format moz-action:ACTION,PARAMS
+          // Where PARAMS is a JSON encoded object.
+          aUrl = decodeURI(aUrl);
+          let [, type, params] = aUrl.match(/^moz-action:([^,]+),(.*)$/);
+
+          let action = {
+            type: type,
+          };
+
+          try {
+            action.params = JSON.parse(params);
+          } catch (e) {
+            // If this failed, we assume that params is not a JSON object, and
+            // is instead just a flat string. This will happen when
+            // UnifiedComplete is disabled - in which case, the param is always
+            // a URL.
+            action.params = {
+              url: params,
+            }
+          }
+
+          return action;
        ]]></body>
      </method>

@ -888,13 +912,13 @@
            var where = whereToOpenLink(aEvent, false, true);
            searchBar.doSearch(search, where);
          }
-          ]]></body>
-        </method>
-      </implementation>
-    </binding>
+        ]]></body>
+      </method>
+    </implementation>
+  </binding>

-    <binding id="urlbar-rich-result-popup" extends="chrome://global/content/bindings/autocomplete.xml#autocomplete-rich-result-popup">
-      <implementation>
+  <binding id="urlbar-rich-result-popup" extends="chrome://global/content/bindings/autocomplete.xml#autocomplete-rich-result-popup">
+    <implementation>
      <field name="_maxResults">0</field>

      <field name="_bundle" readonly="true">
--- a/browser/components/tabview/test/browser_tabview_bug715454.js
+++ b/browser/components/tabview/test/browser_tabview_bug715454.js
@ -64,7 +64,7 @@ function switchToURL(groupItemOne, groupItemTwo) {
   * switch. The tab should be opened in group two and not in group one.
   */
  // Set the urlbar to include the moz-action.
-  gURLBar.value = "moz-action:switchtab,about:mozilla";
+  gURLBar.value = "moz-action:switchtab," + JSON.stringify({url: "about:mozilla"});
  // Focus the urlbar so we can press enter.
  gURLBar.focus();
  // Press enter.
--- a/browser/components/tabview/test/browser_tabview_bug716880.js
+++ b/browser/components/tabview/test/browser_tabview_bug716880.js
@ -67,7 +67,7 @@ function switchToURL(groupItemOne, groupItemTwo) {
   * switch. The selected group should be group one.
   */
  // Set the urlbar to include the moz-action.
-  gURLBar.value = "moz-action:switchtab,about:mozilla";
+  gURLBar.value = "moz-action:switchtab," + JSON.stringify({url: "about:mozilla"});
  // Focus the urlbar so we can press enter.
  gURLBar.focus();
  // Press enter.
--- a/browser/themes/linux/browser.css
+++ b/browser/themes/linux/browser.css
@ -901,7 +901,7 @@ toolbarbutton[sdk-button="true"][cui-areatype="toolbar"] > .toolbarbutton-icon {
  -moz-margin-end: 3px;
 }

-#urlbar-display {
+.urlbar-display {
  margin-top: 0;
  margin-bottom: 0;
  -moz-margin-start: 0;
--- a/browser/themes/osx/browser.css
+++ b/browser/themes/osx/browser.css
@ -2054,7 +2054,7 @@ toolbarbutton[sdk-button="true"][cui-areatype="toolbar"] > .toolbarbutton-icon {
  -moz-margin-end: 3px;
 }

-#urlbar-display {
+.urlbar-display {
  margin-top: 0;
  margin-bottom: 0;
  color: GrayText;
--- a/browser/themes/windows/browser.css
+++ b/browser/themes/windows/browser.css
@ -1247,7 +1247,7 @@ html|*.urlbar-input:-moz-lwtheme::-moz-placeholder,
  -moz-margin-end: 3px;
 }

-#urlbar-display {
+.urlbar-display {
  margin-top: 0;
  margin-bottom: 0;
  -moz-margin-start: 0;
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@ -492,6 +492,23 @@ function stripHttpAndTrim(spec) {
  return spec;
 }

+/**
+ * Make a moz-action: URL for a given action and set of parameters.
+ *
+ * @param action
+ *        Name of the action
+ * @param params
+ *        Object, whose keys are parameter names and values are the
+ *        corresponding parameter values.
+ * @return String representation of the built moz-action: URL
+ */
+function makeActionURL(action, params) {
+  let url = "moz-action:" + action + "," + JSON.stringify(params);
+  // Make a nsIURI out of this to ensure it's encoded properly.
+  return NetUtil.newURI(url).spec;
+}
+
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Search Class
 //// Manages a single instance of an autocomplete search.
@ -934,9 +951,12 @@ Search.prototype = {

    // If actions are enabled and the page is open, add only the switch-to-tab
    // result.  Otherwise, add the normal result.
-    let [url, action] = this._enableActions && openPageCount > 0 ?
-                        ["moz-action:switchtab," + escapedURL, "switchtab"] :
-                        [escapedURL, null];
+    let url = escapedURL;
+    let action = null;
+    if (this._enableActions && openPageCount > 0) {
+      url = makeActionURL("switchtab", {url: escapedURL});
+      action = "switchtab";
+    }

    // Always prefer the bookmark title unless it is empty
    let title = bookmarkTitle || historyTitle;
--- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
+++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
@ -281,3 +281,8 @@ function stripPrefix(spec)
  }
  return spec;
 }
+
+function makeActionURI(action, params) {
+  let url = "moz-action:" + action + "," + JSON.stringify(params);
+  return NetUtil.newURI(url);
+}
--- a/toolkit/components/places/tests/unifiedcomplete/test_tabmatches.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_tabmatches.js
@ -22,14 +22,14 @@ add_task(function* test_tab_matches() {
  yield check_autocomplete({
    search: "abc.com",
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,http://abc.com/"), title: "ABC rocks" } ]
+    matches: [ { uri: makeActionURI("switchtab", {url: "http://abc.com/"}), title: "ABC rocks" } ]
  });

  do_log_info("two results, one tab match");
  yield check_autocomplete({
    search: "abc",
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,http://abc.com/"), title: "ABC rocks" },
+    matches: [ { uri: makeActionURI("switchtab", {url: "http://abc.com/"}), title: "ABC rocks" },
               { uri: uri2, title: "xyz.net - we're better than ABC" } ]
  });

@ -38,8 +38,8 @@ add_task(function* test_tab_matches() {
  yield check_autocomplete({
    search: "abc",
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,http://abc.com/"), title: "ABC rocks" },
-               { uri: NetUtil.newURI("moz-action:switchtab,http://xyz.net/"), title: "xyz.net - we're better than ABC" } ]
+    matches: [ { uri: makeActionURI("switchtab", {url: "http://abc.com/"}), title: "ABC rocks" },
+               { uri: makeActionURI("switchtab", {url: "http://xyz.net/"}), title: "xyz.net - we're better than ABC" } ]
  });

  do_log_info("two results, both tab matches, one has multiple tabs");
@ -47,8 +47,8 @@ add_task(function* test_tab_matches() {
  yield check_autocomplete({
    search: "abc",
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,http://abc.com/"), title: "ABC rocks" },
-               { uri: NetUtil.newURI("moz-action:switchtab,http://xyz.net/"), title: "xyz.net - we're better than ABC" } ]
+    matches: [ { uri: makeActionURI("switchtab", {url: "http://abc.com/"}), title: "ABC rocks" },
+               { uri: makeActionURI("switchtab", {url: "http://xyz.net/"}), title: "xyz.net - we're better than ABC" } ]
  });

  do_log_info("two results, no tab matches");
@ -66,30 +66,30 @@ add_task(function* test_tab_matches() {
  yield check_autocomplete({
    search: gTabRestrictChar + " abc",
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,http://abc.com/"), title: "ABC rocks" } ]
+    matches: [ { uri: makeActionURI("switchtab", {url: "http://abc.com/"}), title: "ABC rocks" } ]
  });

  do_log_info("tab match with not-addable pages");
  yield check_autocomplete({
    search: "mozilla",
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,about:mozilla"), title: "about:mozilla" } ]
+    matches: [ { uri: makeActionURI("switchtab", {url: "about:mozilla"}), title: "about:mozilla" } ]
  });

  do_log_info("tab match with not-addable pages and restriction character");
  yield check_autocomplete({
    search: gTabRestrictChar + " mozilla",
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,about:mozilla"), title: "about:mozilla" } ]
+    matches: [ { uri: makeActionURI("switchtab", {url: "about:mozilla"}), title: "about:mozilla" } ]
  });

  do_log_info("tab match with not-addable pages and only restriction character");
  yield check_autocomplete({
    search: gTabRestrictChar,
    searchParam: "enable-actions",
-    matches: [ { uri: NetUtil.newURI("moz-action:switchtab,http://abc.com/"), title: "ABC rocks" },
-               { uri: NetUtil.newURI("moz-action:switchtab,about:mozilla"), title: "about:mozilla" },
-               { uri: NetUtil.newURI("moz-action:switchtab,data:text/html,test"), title: "data:text/html,test" } ]
+    matches: [ { uri: makeActionURI("switchtab", {url: "http://abc.com/"}), title: "ABC rocks" },
+               { uri: makeActionURI("switchtab", {url: "about:mozilla"}), title: "about:mozilla" },
+               { uri: makeActionURI("switchtab", {url: "data:text/html,test"}), title: "data:text/html,test" } ]
  });

  yield cleanup();
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@ -1445,9 +1445,17 @@ extends="chrome://global/content/bindings/popup.xml#popup">
      <method name="_adjustAcItem">
        <body>
          <![CDATA[
-          var url = this.getAttribute("url");
-          var title = this.getAttribute("title");
-          var type = this.getAttribute("type");
+          let url = this.getAttribute("url");
+          let title = this.getAttribute("title");
+          let type = this.getAttribute("type");
+
+          let emphasiseTitle = true;
+          let emphasiseUrl = true;
+
+          // Hide the title's extra box by default, until we find out later if
+          // we need extra stuff.
+          this._extraBox.hidden = true;
+          this._titleBox.flex = 1;

          this.removeAttribute("actiontype");

@ -1457,34 +1465,36 @@ extends="chrome://global/content/bindings/popup.xml#popup">
          // space when hidden.  Setting the hidden property accomplishes that.
          this._titleOverflowEllipsis.hidden = false;

+          let types = new Set(type.split(/\s+/));
+
          // If the type includes an action, set up the item appropriately.
-          var types = type.split(/\s+/);
-          var actionIndex = types.indexOf("action");
-          if (actionIndex >= 0) {
-            let [,action, param] = url.match(/^moz-action:([^,]+),(.*)$/);
-            this.setAttribute("actiontype", action);
-            url = param;
-            let desc = this._stringBundle.GetStringFromName("switchToTab");
-            this._setUpDescription(this._action, desc, true);
+          if (types.has("action")) {
+            let action = this._parseActionUrl(url);
+            this.setAttribute("actiontype", action.type);
+
+            if (action.type == "switchtab") {
+              url = action.params.url;
+              let desc = this._stringBundle.GetStringFromName("switchToTab");
+              this._setUpDescription(this._action, desc, true);
+            }

            // Remove the "action" substring so that the correct style, if any,
            // is applied below.
-            types.splice(actionIndex, 1);
-            type = types.join(" ");
+            types.delete("action");
          }

          // Check if we have a search engine name
-          let searchEngine = "";
-          let searchIndex = types.indexOf("search");
-          if (searchIndex >= 0) {
+          if (types.has("search")) {
+            emphasiseUrl = false;
+
            const TITLE_SEARCH_ENGINE_SEPARATOR = " \u00B7\u2013\u00B7 ";

            [title, searchEngine] = title.split(TITLE_SEARCH_ENGINE_SEPARATOR);
+            url = this._stringBundle.formatStringFromName("searchWithEngine", [searchEngine], 1);

            // Remove the "search" substring so that the correct style, if any,
            // is applied below.
-            types.splice(searchIndex, 1);
-            type = types.join(" ");
+            types.delete("search");
          }

          // If we have a tag match, show the tags and icon
@ -1531,10 +1541,6 @@ extends="chrome://global/content/bindings/popup.xml#popup">

            // Don't emphasize keyword searches in the title or url
            this.setAttribute("text", "");
-          } else {
-            // Hide the title's extra box if we don't need extra stuff
-            this._extraBox.hidden = true;
-            this._titleBox.flex = 1;
          }

          // Give the image the icon style and a special one for the type
@ -1546,15 +1552,8 @@ extends="chrome://global/content/bindings/popup.xml#popup">
            title = url;

          // Emphasize the matching search terms for the description
-          this._setUpDescription(this._title, title);
-          if (!searchEngine) {
-            this._setUpDescription(this._url, url);
-          } else {
-            let desc = this._stringBundle.formatStringFromName("searchWithEngine", [searchEngine], 1);
-
-            // The search engine name, when present, is not emphasized.
-            this._setUpDescription(this._url, desc, true);
-          }
+          this._setUpDescription(this._title, title, !emphasiseTitle);
+          this._setUpDescription(this._url, url, !emphasiseUrl);

          // Set up overflow on a timeout because the contents of the box
          // might not have a width yet even though we just changed them
@ -1564,6 +1563,37 @@ extends="chrome://global/content/bindings/popup.xml#popup">
        </body>
      </method>

+      <method name="_parseActionUrl">
+        <parameter name="aUrl"/>
+        <body><![CDATA[
+          if (!aUrl.startsWith("moz-action:"))
+            return null;
+
+          // URL is in the format moz-action:ACTION,PARAMS
+          // Where PARAMS is a JSON encoded object.
+          aUrl = decodeURI(aUrl);
+          let [, type, params] = aUrl.match(/^moz-action:([^,]+),(.*)$/);
+
+          let action = {
+            type: type,
+          };
+
+          try {
+            action.params = JSON.parse(params);
+          } catch (e) {
+            // If this failed, we assume that params is not a JSON object, and
+            // is instead just a flat string. This will happen when
+            // UnifiedComplete is disabled - in which case, the param is always
+            // a URL.
+            action.params = {
+              url: params,
+            }
+          }
+
+          return action;
+        ]]></body>
+      </method>
+
      <method name="_setUpOverflow">
        <parameter name="aParentBox"/>
        <parameter name="aEllipsis"/>