Bug 566489 - Enable inline autocomplete again, but make it smarter for URLs r=sdwilsh sr=gavin a=sheriff

browser/app/profile/firefox.js

@@ -264,7 +264,7 @@ pref("browser.urlbar.doubleClickSelectsAll", true);
 #else
 pref("browser.urlbar.doubleClickSelectsAll", false);
 #endif
-pref("browser.urlbar.autoFill", false);
+pref("browser.urlbar.autoFill", true);
 // 0: Match anywhere (e.g., middle of words)
 // 1: Match on word boundaries and then try matching anywhere
 // 2: Match only on word boundaries (e.g., after / or .)

toolkit/components/autocomplete/nsAutoCompleteController.cpp

@@ -24,6 +24,7 @@
  *   Dean Tessman <dean_tessman@hotmail.com>
  *   Johnny Stenback <jst@mozilla.jstenback.com>
  *   Masayuki Nakano <masayuki@d-toybox.com>
+ *   Michael Ventnor <m.ventnor@gmail.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -54,6 +55,7 @@
 #include "nsIDOMKeyEvent.h"
 #include "mozilla/Services.h"
 #include "mozilla/ModuleUtils.h"
+#include "mozilla/Util.h"
 
 static const char *kAutoCompleteSearchCID = "@mozilla.org/autocomplete/search;1?name=";
 
@@ -94,6 +96,43 @@ nsAutoCompleteController::~nsAutoCompleteController()
   SetInput(nsnull);
 }
 
+////////////////////////////////////////////////////////////////////////
+//// Helper methods
+
+/**
+ * Cuts any URL prefixes from a given string, making it suitable for search
+ * comparisons.
+ *
+ * @param aOrigSpec
+ *        The string to look for prefixes in.
+ * @return A substring, with any URL prefixes removed, that depends on the
+ *         same buffer as aOrigSpec (to save allocations).
+ */
+static const nsDependentSubstring
+RemoveURIPrefixes(const nsAString &aOrigSpec)
+{
+  nsDependentSubstring result(aOrigSpec, 0);
+
+  if (StringBeginsWith(result, NS_LITERAL_STRING("moz-action:"))) {
+    PRUint32 locationOfComma = result.FindChar(',', 11);
+    result.Rebind(result, locationOfComma + 1);
+  }
+
+  if (StringBeginsWith(result, NS_LITERAL_STRING("http://"))) {
+    result.Rebind(result, 7);
+  } else if (StringBeginsWith(result, NS_LITERAL_STRING("https://"))) {
+    result.Rebind(result, 8);
+  } else if (StringBeginsWith(result, NS_LITERAL_STRING("ftp://"))) {
+    result.Rebind(result, 6);
+  }
+
+  if (StringBeginsWith(result, NS_LITERAL_STRING("www."))) {
+    result.Rebind(result, 4);
+  }
+
+  return result;
+}
+
 ////////////////////////////////////////////////////////////////////////
 //// nsIAutoCompleteController
 
@@ -1393,6 +1432,64 @@ nsAutoCompleteController::CompleteDefaultIndex(PRInt32 aSearchIndex)
   return NS_OK;
 }
 
+nsresult
+nsAutoCompleteController::GetDefaultCompleteURLValue(nsIAutoCompleteResult *aResult,
+                                                     PRBool aPreserveCasing,
+                                                     nsAString &_retval)
+{
+  MOZ_ASSERT(aResult);
+
+  PRUint32 rowCount;
+  (void)aResult->GetMatchCount(&rowCount);
+  if (rowCount == 0) {
+    // Return early if we have no entries, so that we don't waste time
+    // fixing up mSearchString below
+    return NS_ERROR_FAILURE;
+  }
+
+  const nsDependentSubstring& fixedSearchTerm = RemoveURIPrefixes(mSearchString);
+  for (PRUint32 i = 0; i < rowCount; ++i) {
+    nsAutoString resultValue;
+    aResult->GetValueAt(i, resultValue);
+    const nsDependentSubstring& fixedResult = RemoveURIPrefixes(resultValue);
+
+    if (!StringBeginsWith(fixedResult, fixedSearchTerm,
+                          nsCaseInsensitiveStringComparator())) {
+      // Not a matching URL
+      continue;
+    }
+
+    // Found a matching item!  Figure out what needs to be assigned/appended.
+    if (aPreserveCasing) {
+      // Use nsDependentString here so we have access to FindCharInSet.
+      const nsDependentString appendValue(Substring(fixedResult, fixedSearchTerm.Length()));
+
+      // We only want to autocomplete up to the next separator.  This lets a user
+      // go to a toplevel domain, if a longer path in that domain is higher in
+      // the autocomplete.
+      // eg. if the user types "m" and "mozilla.org/credits" is the top hit,
+      // autocomplete only to "mozilla.org/" in case that's where they want to go.
+      // They're one keystroke away from "/credits", anyway.
+      PRInt32 separatorIndex = appendValue.FindCharInSet("/?#");
+      if (separatorIndex != kNotFound && appendValue[separatorIndex] == '/') {
+        // Add 1 so we include the directory separator
+        separatorIndex++;
+      }
+
+      nsAutoString returnValue;
+      returnValue.Assign(mSearchString);
+      returnValue.Append(Substring(appendValue, 0, separatorIndex));
+      _retval = returnValue;
+    } else {
+      _retval.Assign(fixedResult);
+    }
+    return NS_OK;
+  }
+
+  // No match at all
+  return NS_ERROR_FAILURE;
+}
+
 nsresult
 nsAutoCompleteController::GetDefaultCompleteValue(PRInt32 aSearchIndex,
                                                   PRBool aPreserveCasing,
@@ -1416,6 +1513,14 @@ nsAutoCompleteController::GetDefaultCompleteValue(PRInt32 aSearchIndex,
   nsIAutoCompleteResult *result = mResults.SafeObjectAt(index);
   NS_ENSURE_TRUE(result != nsnull, NS_ERROR_FAILURE);
 
+  PRBool isURL;
+  result->GetIsURLResult(&isURL);
+  if (isURL) {
+    // For URLs, we remove needless prefixes, then iterate over all values
+    // to find a suitable default.
+    return GetDefaultCompleteURLValue(result, aPreserveCasing, _retval);
+  }
+
   if (defaultIndex < 0) {
     // The search must explicitly provide a default index in order
     // for us to be able to complete.
@@ -1466,37 +1571,11 @@ nsAutoCompleteController::CompleteValue(nsString &aValue)
     // autocomplete to aValue.
     mInput->SetTextValue(aValue);
   } else {
-    nsresult rv;
-    nsCOMPtr<nsIIOService> ios = do_GetService(NS_IOSERVICE_CONTRACTID, &rv);
-    NS_ENSURE_SUCCESS(rv, rv);
-    nsCAutoString scheme;
-    if (NS_SUCCEEDED(ios->ExtractScheme(NS_ConvertUTF16toUTF8(aValue), scheme))) {
-      // Trying to autocomplete a URI from somewhere other than the beginning.
-      // Only succeed if the missing portion is "http://"; otherwise do not
-      // autocomplete.  This prevents us from "helpfully" autocompleting to a
-      // URI that isn't equivalent to what the user expected.
-      const PRInt32 findIndex = 7; // length of "http://"
-
-      if ((endSelect < findIndex + mSearchStringLength) ||
-          !scheme.LowerCaseEqualsLiteral("http") ||
-          !Substring(aValue, findIndex, mSearchStringLength).Equals(
-            mSearchString, nsCaseInsensitiveStringComparator())) {
-        return NS_OK;
-      }
-
-      mInput->SetTextValue(mSearchString +
-                           Substring(aValue, mSearchStringLength + findIndex,
-                                     endSelect));
-
-      endSelect -= findIndex; // We're skipping this many characters of aValue.
-    } else {
-      // Autocompleting something other than a URI from the middle.
-      // Use the format "searchstring >> full string" to indicate to the user
-      // what we are going to replace their search string with.
-      mInput->SetTextValue(mSearchString + NS_LITERAL_STRING(" >> ") + aValue);
-
-      endSelect = mSearchString.Length() + 4 + aValue.Length();
-    }
+    // Autocompleting something from the middle.
+    // Use the format "searchstring >> full string" to indicate to the user
+    // what we are going to replace their search string with.
+    mInput->SetTextValue(mSearchString + NS_LITERAL_STRING(" >> ") + aValue);
+    endSelect = mSearchString.Length() + 4 + aValue.Length();
   }
 
   mInput->SelectTextRange(mSearchStringLength, endSelect);

toolkit/components/autocomplete/nsAutoCompleteController.h

@@ -97,6 +97,21 @@ protected:
 private:
   nsresult GetResultValueLabelAt(PRInt32 aIndex, PRBool aValueOnly,
                                  PRBool aGetValue, nsAString & _retval);
+  /**
+   * Searches for a suitable value to complete to, by comparing all values
+   * as URLs, and completing only up to the next URL separator.
+   *
+   * @param aResult
+   *        An autocomplete result to search in.
+   * @param aPreserveCasing
+   *        Preserve the casing of what the user typed in.
+   * @param [out] _retval
+   *        The value to complete to.
+   * @return A result, NS_OK if there is a value to complete to.
+   */
+  nsresult GetDefaultCompleteURLValue(nsIAutoCompleteResult *aResult,
+                                      PRBool aPreserveCasing,
+                                      nsAString &_retval);
 protected:
   nsresult GetDefaultCompleteValue(PRInt32 aSearchIndex, PRBool aPreserveCasing,
                                    nsAString &_retval);

toolkit/components/autocomplete/nsAutoCompleteSimpleResult.cpp

@@ -43,7 +43,8 @@ NS_IMPL_ISUPPORTS2(nsAutoCompleteSimpleResult,
 
 nsAutoCompleteSimpleResult::nsAutoCompleteSimpleResult() :
   mDefaultIndex(-1),
-  mSearchResult(RESULT_NOMATCH)
+  mSearchResult(RESULT_NOMATCH),
+  mIsURLResult(PR_FALSE)
 {
 }
 
@@ -104,6 +105,21 @@ nsAutoCompleteSimpleResult::SetErrorDescription(
   return NS_OK;
 }
 
+// isURLResult
+NS_IMETHODIMP
+nsAutoCompleteSimpleResult::GetIsURLResult(PRBool *aIsURLResult)
+{
+  *aIsURLResult = mIsURLResult;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsAutoCompleteSimpleResult::SetIsURLResult(PRBool aIsURLResult)
+{
+  mIsURLResult = aIsURLResult;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsAutoCompleteSimpleResult::AppendMatch(const nsAString& aValue,
                                         const nsAString& aComment,

toolkit/components/autocomplete/nsAutoCompleteSimpleResult.h

@@ -78,6 +78,8 @@ protected:
   PRInt32 mDefaultIndex;
   PRUint32 mSearchResult;
 
+  PRBool mIsURLResult;
+
   nsCOMPtr<nsIAutoCompleteSimpleResultListener> mListener;
 };
 

toolkit/components/autocomplete/nsIAutoCompleteResult.idl

@@ -82,6 +82,14 @@ interface nsIAutoCompleteResult : nsISupports
    */
   readonly attribute unsigned long matchCount;
 
+  /**
+   * Whether the values in this result are URLs. This will cause
+   * slightly different behaviour where defaultIndex is intelligently chosen
+   * for you (while the attribute defaultIndex is ignored), and
+   * searching is fine-tuned for URLs.
+   */
+  readonly attribute boolean isURLResult;
+
   /**
    * Get the value of the result at the given index
    */

toolkit/components/autocomplete/nsIAutoCompleteSimpleResult.idl

@@ -74,6 +74,12 @@ interface nsIAutoCompleteSimpleResult : nsIAutoCompleteResult
    */
   void setSearchResult(in unsigned short aSearchResult);
 
+  /**
+   * A writer for the readonly attribute 'isURLResult'.
+   * Sets whether the values in this result are URLs.
+   */
+  void setIsURLResult(in boolean aIsURLResult);
+
   /**
    * Appends a result item consisting of the given value, comment, image and style.
    * This is how you add results.  Note:  image and style are optional. 

toolkit/components/autocomplete/tests/unit/test_autofill.js

@@ -0,0 +1,344 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+/**
+ * Unit test for Bug 566489 - Inline Autocomplete.
+ */
+
+
+/**
+ * Dummy nsIAutoCompleteInput source that returns
+ * the given list of AutoCompleteSearch names. 
+ */
+function AutoCompleteInput(aSearches) {
+  this.searches = aSearches;
+}
+AutoCompleteInput.prototype = {
+  constructor: AutoCompleteInput, 
+  
+  // Array of AutoCompleteSearch names
+  searches: null,
+  
+  minResultsForPopup: 0,
+  timeout: 10,
+  searchParam: "",
+  textValue: "",
+  disableAutoComplete: false,  
+  completeDefaultIndex: true,
+
+  // Text selection range
+  selStart: 0,
+  selEnd: 0,
+  get selectionStart() {
+    return selStart;
+  },
+  get selectionEnd() {
+    return selEnd;
+  },
+  selectTextRange: function(aStart, aEnd) {
+    selStart = aStart;
+    selEnd = aEnd;
+  },
+  
+  get searchCount() {
+    return this.searches.length;
+  },
+  
+  getSearchAt: function(aIndex) {
+    return this.searches[aIndex];
+  },
+  
+  onSearchBegin: function() {},
+  onSearchComplete: function() {},
+  
+  popupOpen: false,  
+  
+  popup: { 
+    setSelectedIndex: function(aIndex) {},
+    invalidate: function() {},
+
+    // nsISupports implementation
+    QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports,
+                                           Ci.nsIAutoCompletePopup])
+  },
+    
+  // nsISupports implementation
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports,
+                                         Ci.nsIAutoCompleteInput])
+}
+
+
+
+/** 
+ * nsIAutoCompleteResult implementation
+ */
+function AutoCompleteResult(aValues, aComments, aStyles) {
+  this._values = aValues;
+  this._comments = aComments;
+  this._styles = aStyles;
+}
+AutoCompleteResult.prototype = {
+  constructor: AutoCompleteResult,
+  
+  // Arrays
+  _values: null,
+  _comments: null,
+  _styles: null,
+  
+  searchString: "",
+  searchResult: Ci.nsIAutoCompleteResult.RESULT_SUCCESS,
+  
+  defaultIndex: 0,
+  isURLResult: true,
+
+  get matchCount() {
+    return this._values.length;
+  },
+
+  getValueAt: function(aIndex) {
+    return this._values[aIndex];
+  },
+
+  getLabelAt: function(aIndex) {
+    return this.getValueAt(aIndex);
+  },
+  
+  getCommentAt: function(aIndex) {
+    return this._comments[aIndex];
+  },
+  
+  getStyleAt: function(aIndex) {
+    return this._styles[aIndex];
+  },
+  
+  getImageAt: function(aIndex) {
+    return "";
+  },
+
+  removeValueAt: function (aRowIndex, aRemoveFromDb) {},
+
+  // nsISupports implementation
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports,
+                                         Ci.nsIAutoCompleteResult])
+}
+
+
+/** 
+ * nsIAutoCompleteSearch implementation that always returns
+ * the same result set.
+ */
+function AutoCompleteSearch(aName, aResult) {
+  this.name = aName;
+  this._result = aResult;
+}
+AutoCompleteSearch.prototype = {
+  constructor: AutoCompleteSearch,
+  
+  // Search name. Used by AutoCompleteController
+  name: null,
+
+  // AutoCompleteResult
+  _result: null,
+
+  
+  /**
+   * Return the same result set for every search
+   */
+  startSearch: function(aSearchString, 
+                        aSearchParam, 
+                        aPreviousResult, 
+                        aListener) 
+  {
+    aListener.onSearchResult(this, this._result);
+  },
+  
+  stopSearch: function() {},
+
+  // nsISupports implementation
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports,
+                                         Ci.nsIFactory,
+                                         Ci.nsIAutoCompleteSearch]),
+  
+  // nsIFactory implementation
+  createInstance: function(outer, iid) {
+    return this.QueryInterface(iid);
+  }
+}
+
+
+/** 
+ * Helper to register an AutoCompleteSearch with the given name.
+ * Allows the AutoCompleteController to find the search.
+ */
+function registerAutoCompleteSearch(aSearch) {
+  var name = "@mozilla.org/autocomplete/search;1?name=" + aSearch.name;
+
+  var uuidGenerator = Cc["@mozilla.org/uuid-generator;1"].
+                      getService(Ci.nsIUUIDGenerator);
+  var cid = uuidGenerator.generateUUID();
+
+  var desc = "Test AutoCompleteSearch";
+  
+  var componentManager = Components.manager
+                                   .QueryInterface(Ci.nsIComponentRegistrar);
+  componentManager.registerFactory(cid, desc, name, aSearch);
+
+  // Keep the id on the object so we can unregister later
+  aSearch.cid = cid; 
+}
+
+
+/** 
+ * Helper to unregister an AutoCompleteSearch. 
+ */
+function unregisterAutoCompleteSearch(aSearch) {
+  var componentManager = Components.manager
+                                   .QueryInterface(Ci.nsIComponentRegistrar);  
+  componentManager.unregisterFactory(aSearch.cid, aSearch);
+}
+
+
+/**
+ * The array of autocomplete test data to run.
+ */
+var tests = [
+  {
+    searchValues: ["mozilla.org"],        // Autocomplete results
+    inputString: "moz",                   // The search string
+    expectedAutocomplete: "mozilla.org",  // The string we expect to be autocompleted to
+    expectedSelStart: 3,                  // The range of the selection we expect
+    expectedSelEnd: 11
+  },
+  {
+    // Test URL schemes
+    searchValues: ["http://www.mozilla.org", "mozNotFirstMatch.org"],
+    inputString: "moz",
+    expectedAutocomplete: "mozilla.org",
+    expectedSelStart: 3,
+    expectedSelEnd: 11
+  },
+  {
+    // Test URL schemes
+    searchValues: ["ftp://ftp.mozilla.org/"],
+    inputString: "ft",
+    expectedAutocomplete: "ftp.mozilla.org/",
+    expectedSelStart: 2,
+    expectedSelEnd: 16
+  },
+  {
+    // Test the moz-action scheme, used internally for things like switch-to-tab
+    searchValues: ["moz-action:someaction,http://www.mozilla.org", "mozNotFirstMatch.org"],
+    inputString: "moz",
+    expectedAutocomplete: "mozilla.org",
+    expectedSelStart: 3,
+    expectedSelEnd: 11
+  },
+  {
+    // Test that we autocomplete to the first match, not necessarily the first entry
+    searchValues: ["unimportantTLD.org/moz", "mozilla.org"],
+    inputString: "moz",
+    expectedAutocomplete: "mozilla.org",
+    expectedSelStart: 3,
+    expectedSelEnd: 11
+  },
+  {
+    // Test that we only autocomplete to the next URL separator (/)
+    searchValues: ["http://mozilla.org/credits/morecredits"],
+    inputString: "moz",
+    expectedAutocomplete: "mozilla.org/",
+    expectedSelStart: 3,
+    expectedSelEnd: 12
+  },
+  {
+    // Test that we only autocomplete to the next URL separator (/)
+    searchValues: ["http://mozilla.org/credits/morecredits"],
+    inputString: "mozilla.org/cr",
+    expectedAutocomplete: "mozilla.org/credits/",
+    expectedSelStart: 14,
+    expectedSelEnd: 20
+  },
+  {
+    // Test that we only autocomplete to before the next URL separator (#)
+    searchValues: ["http://mozilla.org/credits#VENTNOR"],
+    inputString: "mozilla.org/cr",
+    expectedAutocomplete: "mozilla.org/credits",
+    expectedSelStart: 14,
+    expectedSelEnd: 19
+  },
+  {
+    // Test that we only autocomplete to before the next URL separator (?)
+    searchValues: ["http://mozilla.org/credits?mozilla=awesome"],
+    inputString: "mozilla.org/cr",
+    expectedAutocomplete: "mozilla.org/credits",
+    expectedSelStart: 14,
+    expectedSelEnd: 19
+  },
+  {
+    // Test that schemes are removed from the input
+    searchValues: ["http://www.mozilla.org/credits"],
+    inputString: "http://mozi",
+    expectedAutocomplete: "http://mozilla.org/",
+    expectedSelStart: 11,
+    expectedSelEnd: 19
+  },
+];
+
+
+/**
+ * Run an individual autocomplete search, one at a time.
+ */
+function run_search() {
+  if (tests.length == 0) {
+    do_test_finished();
+    return;
+  }
+
+  var test = tests.shift();
+
+  var search = new AutoCompleteSearch("test-autofill1",
+    new AutoCompleteResult(test.searchValues, ["", ""], ["", ""]));
+
+  // Register search so AutoCompleteController can find them
+  registerAutoCompleteSearch(search);
+
+  var controller = Cc["@mozilla.org/autocomplete/controller;1"].
+                   getService(Ci.nsIAutoCompleteController);
+
+  // Make an AutoCompleteInput that uses our search
+  // and confirms results on search complete
+  var input = new AutoCompleteInput([search.name]);
+  input.textValue = test.inputString;
+
+  // Caret must be at the end. Autofill doesn't happen unless you're typing
+  // characters at the end.
+  var strLen = test.inputString.length;
+  input.selectTextRange(strLen, strLen);
+
+  input.onSearchComplete = function() {
+    do_check_eq(input.textValue, test.expectedAutocomplete);
+    do_check_eq(input.selectionStart, test.expectedSelStart);
+    do_check_eq(input.selectionEnd, test.expectedSelEnd);
+
+    // Unregister searches
+    unregisterAutoCompleteSearch(search);
+    run_search();
+  };
+
+  controller.input = input;
+  controller.startSearch(test.inputString);
+}
+
+
+/** 
+ */
+function run_test() {
+  // Search is asynchronous, so don't let the test finish immediately
+  do_test_pending();
+  run_search();
+}
+

toolkit/components/filepicker/nsFileView.cpp

@@ -154,6 +154,13 @@ NS_IMETHODIMP nsFileResult::GetErrorDescription(nsAString & aErrorDescription)
   return NS_OK;
 }
 
+NS_IMETHODIMP nsFileResult::GetIsURLResult(PRBool *aIsURLResult)
+{
+  NS_ENSURE_ARG_POINTER(aIsURLResult);
+  *aIsURLResult = PR_FALSE;
+  return NS_OK;
+}
+
 NS_IMETHODIMP nsFileResult::GetMatchCount(PRUint32 *aMatchCount)
 {
   NS_ENSURE_ARG_POINTER(aMatchCount);

toolkit/components/places/nsPlacesAutoComplete.js

@@ -458,6 +458,7 @@ nsPlacesAutoComplete.prototype = {
                  createInstance(Ci.nsIAutoCompleteSimpleResult);
     result.setSearchString(aSearchString);
     result.setListener(this);
+    result.setIsURLResult(true);
     this._result = result;
 
     // If we are not enabled, we need to return now.
@@ -737,7 +738,6 @@ nsPlacesAutoComplete.prototype = {
     if (aSearchOngoing)
       resultCode += "_ONGOING";
     result.setSearchResult(Ci.nsIAutoCompleteResult[resultCode]);
-    result.setDefaultIndex(result.matchCount ? 0 : -1);
     this._listener.onSearchResult(this, result);
   },