Bug 564573 - A page that's opened in a tab should appear exactly once, as switch-to-tab. r=mak

toolkit/components/places/src/SQLFunctions.cpp

@@ -234,6 +234,7 @@ namespace places {
     bool bookmark = aArguments->AsInt32(kArgIndexBookmark) ? true : false;
     nsAutoString tags;
     (void)aArguments->GetString(kArgIndexTags, tags);
+    PRInt32 openPageCount = aArguments->AsInt32(kArgIndexOpenPageCount);
 
     // Make sure we match all the filter requirements.  If a given restriction
     // is active, make sure the corresponding condition is not true.
@@ -241,7 +242,8 @@ namespace places {
       (HAS_BEHAVIOR(HISTORY) && visitCount == 0) ||
       (HAS_BEHAVIOR(TYPED) && !typed) ||
       (HAS_BEHAVIOR(BOOKMARK) && !bookmark) ||
-      (HAS_BEHAVIOR(TAG) && tags.IsVoid())
+      (HAS_BEHAVIOR(TAG) && tags.IsVoid()) ||
+      (HAS_BEHAVIOR(OPENPAGE) && openPageCount == 0)
     );
     if (!matches) {
       NS_IF_ADDREF(*_result = new IntegerVariant(0));

toolkit/components/places/src/SQLFunctions.h

@@ -60,7 +60,8 @@ namespace places {
  *
  * In SQL, you'd use it in the WHERE clause like so:
  * WHERE AUTOCOMPLETE_MATCH(aSearchString, aURL, aTitle, aTags, aVisitCount,
- *                          aTyped)
+ *                          aTyped, aBookmark, aOpenPageCount, aMatchBehavior,
+ *                          aSearchBehavior)
  *
  * @param aSearchString
  *        The string to compare against.
@@ -76,6 +77,9 @@ namespace places {
  *        Indicates if aURL is a typed URL or not.  Treated as a boolean.
  * @param aBookmark
  *        Indicates if aURL is a bookmark or not.  Treated as a boolean.
+ * @param aOpenPageCount
+ *        The number of times aURL has been registered as being open.  (See
+ *        nsIBrowserHistory.registerOpenPage.)
  * @param aMatchBehavior
  *        The match behavior to use for this search.
  * @param aSearchBehavior
@@ -106,9 +110,10 @@ private:
   static const PRUint32 kArgIndexVisitCount = 4;
   static const PRUint32 kArgIndexTyped = 5;
   static const PRUint32 kArgIndexBookmark = 6;
-  static const PRUint32 kArgIndexMatchBehavior = 7;
-  static const PRUint32 kArgIndexSearchBehavior = 8;
-  static const PRUint32 kArgIndexLength = 9;
+  static const PRUint32 kArgIndexOpenPageCount = 7;
+  static const PRUint32 kArgIndexMatchBehavior = 8;
+  static const PRUint32 kArgIndexSearchBehavior = 9;
+  static const PRUint32 kArgIndexLength = 10;
 
   /**
    * Typedefs

toolkit/components/places/src/nsPlacesAutoComplete.js

@@ -227,6 +227,7 @@ function nsPlacesAutoComplete()
            "AND AUTOCOMPLETE_MATCH(:searchString, h.url, " +
                                   "IFNULL(bookmark, h.title), tags, " +
                                   "h.visit_count, h.typed, parent, " +
+                                  "t.open_count, " +
                                   ":matchBehavior, :searchBehavior) " +
           "{ADDITIONAL_CONDITIONS} ";
   }
@@ -338,6 +339,7 @@ function nsPlacesAutoComplete()
       "AND AUTOCOMPLETE_MATCH(:searchString, c_url, " +
                              "IFNULL(bookmark, c_title), tags, " +
                              "c_visit_count, c_typed, parent, " +
+                             "t.open_count, " +
                              ":matchBehavior, :searchBehavior) " +
       "ORDER BY rank DESC, IFNULL(h_t.frecency, h.frecency) DESC"
     );
@@ -702,9 +704,11 @@ nsPlacesAutoComplete.prototype = {
     this._matchURLToken = safeGetter("match.url", "@");
     this._defaultBehavior = safeGetter("default.behavior", 0);
     // Further restrictions to apply for "empty searches" (i.e. searches for "").
-    // By default we use (HISTORY | TYPED | OPENPAGE) = 161.
-    this._emptySearchDefaultBehavior = this._defaultBehavior |
-                                       safeGetter("default.behavior.emptyRestriction", 161);
+    this._emptySearchDefaultBehavior =
+      this._defaultBehavior |
+      safeGetter("default.behavior.emptyRestriction",
+                 Ci.mozIPlacesAutoComplete.BEHAVIOR_HISTORY |
+                 Ci.mozIPlacesAutoComplete.BEHAVIOR_TYPED);
 
     // Validate matchBehavior; default to MATCH_BOUNDARY_ANYWHERE.
     if (this._matchBehavior != MATCH_ANYWHERE &&
@@ -951,22 +955,12 @@ nsPlacesAutoComplete.prototype = {
         style = "favicon";
     }
 
-    // If actions aren't enabled, avoid doing any additional work.
-    if (!this._enableActions) {
-      this._addToResults(entryId, escapedEntryURL, title, entryFavicon, style);
-      return true;
-    }
-
-    // Add a special entry for an open-page match.
-    if ((this._hasBehavior("openpage") || this._hasBehavior("everything")) &&
-        openPageCount > 0)
-      this._addToResults(entryId, "moz-action:switchtab," + escapedEntryURL, title, entryFavicon, "action");
-
-    // If restricting to only open-page matches, there should only be the
-    // switch-to-tab results.
-    if (!this._onlyHasBehavior("openpage"))
-      this._addToResults(entryId, escapedEntryURL, title, entryFavicon, style);
-
+    // If actions are enabled and the page is open, add only the switch-to-tab
+    // result.  Otherwise, add the normal result.
+    let [url, style] = this._enableActions && openPageCount > 0 ?
+                       ["moz-action:switchtab," + escapedEntryURL, "action"] :
+                       [escapedEntryURL, style];
+    this._addToResults(entryId, url, title, entryFavicon, style);
     return true;
   },
 
@@ -1020,32 +1014,15 @@ nsPlacesAutoComplete.prototype = {
    * Determines if the specified AutoComplete behavior is set.
    *
    * @param aType
-   *        The behavior type to test for, or "everything" to test if no
-   *        specific behavior has been set.
+   *        The behavior type to test for.
    * @return true if the behavior is set, false otherwise.
    */
   _hasBehavior: function PAC_hasBehavior(aType)
   {
-    if (aType == "everything")
-      return this._behavior == 0;
     return (this._behavior &
             Ci.mozIPlacesAutoComplete["BEHAVIOR_" + aType.toUpperCase()]);
   },
 
-  /**
-   * Determines if the specified AutoComplete behavior is the only behavior set.
-   *
-   * @param aType
-   *        The behavior type to test for.
-   * @return true if the behavior is set and no other behaviors are set,
-   *         false otherwise.
-   */
-  _onlyHasBehavior: function PAC_onlyHasBehavior(aType)
-  {
-    return (this._behavior ==
-            Ci.mozIPlacesAutoComplete["BEHAVIOR_" + aType.toUpperCase()]);
-  },
-
   /**
    * Enables the desired AutoComplete behavior.
    *

toolkit/components/places/tests/autocomplete/test_tabmatches.js

@@ -62,16 +62,16 @@ addOpenPages(0, 1);
 
 let gTests = [
   ["0: single result, that is also a tab match",
-   "abc.com", [0,1]],
+   "abc.com", [1]],
   ["1: two results, one tab match",
-   "abc", [0,1,2]],
+   "abc", [1,2]],
   ["2: two results, both tab matches",
-   "abc", [0,1,2,3],
+   "abc", [1,3],
    function() {
      addOpenPages(2, 1);
    }],
   ["3: two results, both tab matches, one has multiple tabs",
-   "abc", [0,1,2,3],
+   "abc", [1,3],
    function() {
      addOpenPages(2, 5);
    }],