diff --git a/mobile/android/base/home/BrowserSearch.java b/mobile/android/base/home/BrowserSearch.java index eb1106b5359d..18c6c0644416 100644 --- a/mobile/android/base/home/BrowserSearch.java +++ b/mobile/android/base/home/BrowserSearch.java @@ -325,19 +325,64 @@ public class BrowserSearch extends HomeFragment } private void handleAutocomplete(String searchTerm, Cursor c) { - if (TextUtils.isEmpty(mSearchTerm) || c == null || mAutocompleteHandler == null) { + if (c == null || + mAutocompleteHandler == null || + TextUtils.isEmpty(mSearchTerm)) { return; } // Avoid searching the path if we don't have to. Currently just - // decided by if there is a '/' character in the string. - final boolean searchPath = (searchTerm.indexOf("/") > 0); + // decided by whether there is a '/' character in the string. + final boolean searchPath = searchTerm.indexOf('/') > 0; final String autocompletion = findAutocompletion(searchTerm, c, searchPath); - if (autocompletion != null && mAutocompleteHandler != null) { - mAutocompleteHandler.onAutocomplete(autocompletion); - mAutocompleteHandler = null; + if (autocompletion == null || mAutocompleteHandler == null) { + return; } + + mAutocompleteHandler.onAutocomplete(autocompletion); + mAutocompleteHandler = null; + } + + /** + * Returns the substring of a provided URI, starting at the given offset, + * and extending up to the end of the path segment in which the provided + * index is found. + * + * For example, given + * + * "www.reddit.com/r/boop/abcdef", 0, ? + * + * this method returns + * + * ?=2: "www.reddit.com/" + * ?=17: "www.reddit.com/r/boop/" + * ?=21: "www.reddit.com/r/boop/" + * ?=22: "www.reddit.com/r/boop/abcdef" + * + */ + private static String uriSubstringUpToMatchedPath(final String url, final int offset, final int begin) { + final int afterEnd = url.length(); + + // We want to include the trailing slash, but not other characters. + int chop = url.indexOf('/', begin); + if (chop != -1) { + ++chop; + if (chop < offset) { + // This isn't supposed to happen. Fall back to returning the whole damn thing. + return url; + } + } else { + chop = url.indexOf('?', begin); + if (chop == -1) { + chop = url.indexOf('#', begin); + } + if (chop == -1) { + chop = afterEnd; + } + } + + return url.substring(offset, chop); } private String findAutocompletion(String searchTerm, Cursor c, boolean searchPath) { @@ -345,36 +390,57 @@ public class BrowserSearch extends HomeFragment return null; } + final int searchLength = searchTerm.length(); final int urlIndex = c.getColumnIndexOrThrow(URLColumns.URL); int searchCount = 0; do { - final Uri url = Uri.parse(c.getString(urlIndex)); - final String host = StringUtils.stripCommonSubdomains(url.getHost()); + final String url = c.getString(urlIndex); - // Host may be null for about pages + // Does the completion match against the whole URL? This will match + // about: pages, as well as user input including "http://...". + if (url.startsWith(searchTerm)) { + return uriSubstringUpToMatchedPath(url, 0, searchLength); + } + + final Uri uri = Uri.parse(url); + final String host = uri.getHost(); + + // Host may be null for about pages. if (host == null) { continue; } - final StringBuilder hostBuilder = new StringBuilder(host); - if (hostBuilder.indexOf(searchTerm) == 0) { - return hostBuilder.append("/").toString(); + if (host.startsWith(searchTerm)) { + return host + "/"; } - if (searchPath) { - final List path = url.getPathSegments(); - - for (String s : path) { - hostBuilder.append("/").append(s); - - if (hostBuilder.indexOf(searchTerm) == 0) { - return hostBuilder.append("/").toString(); - } - } + final String strippedHost = StringUtils.stripCommonSubdomains(host); + if (strippedHost.startsWith(searchTerm)) { + return strippedHost + "/"; } - searchCount++; + ++searchCount; + + if (!searchPath) { + continue; + } + + // Otherwise, if we're matching paths, let's compare against the string itself. + final int hostOffset = url.indexOf(strippedHost); + if (hostOffset == -1) { + // This was a URL string that parsed to a different host (normalized?). + // Give up. + continue; + } + + // We already matched the non-stripped host, so now we're + // substring-searching in the part of the URL without the common + // subdomains. + if (url.startsWith(searchTerm, hostOffset)) { + // Great! Return including the rest of the path segment. + return uriSubstringUpToMatchedPath(url, hostOffset, hostOffset + searchLength); + } } while (searchCount < MAX_AUTOCOMPLETE_SEARCH && c.moveToNext()); return null; @@ -787,7 +853,7 @@ public class BrowserSearch extends HomeFragment mAdapter.swapCursor(c); // We should handle autocompletion based on the search term - // associated with the currently loader that has just provided + // associated with the loader that has just provided // the results. SearchCursorLoader searchLoader = (SearchCursorLoader) loader; handleAutocomplete(searchLoader.getSearchTerm(), c); diff --git a/mobile/android/base/tests/testInputUrlBar.java b/mobile/android/base/tests/testInputUrlBar.java index ee1da7f8e87b..0943727cf5cf 100644 --- a/mobile/android/base/tests/testInputUrlBar.java +++ b/mobile/android/base/tests/testInputUrlBar.java @@ -21,25 +21,27 @@ public final class testInputUrlBar extends BaseTest { startEditingMode(); assertUrlBarText("about:home"); - mActions.sendKeys("ab"); - assertUrlBarText("ab"); + // Avoid any auto domain completion by using a prefix that matches + // nothing, including about: pages + mActions.sendKeys("zy"); + assertUrlBarText("zy"); mActions.sendKeys("cd"); - assertUrlBarText("abcd"); + assertUrlBarText("zycd"); mActions.sendSpecialKey(Actions.SpecialKey.LEFT); mActions.sendSpecialKey(Actions.SpecialKey.LEFT); // Inserting "" should not do anything. mActions.sendKeys(""); - assertUrlBarText("abcd"); + assertUrlBarText("zycd"); mActions.sendKeys("ef"); - assertUrlBarText("abefcd"); + assertUrlBarText("zyefcd"); mActions.sendSpecialKey(Actions.SpecialKey.RIGHT); mActions.sendKeys("gh"); - assertUrlBarText("abefcghd"); + assertUrlBarText("zyefcghd"); final EditText editText = mUrlBarEditView; runOnUiThreadSync(new Runnable() { @@ -49,7 +51,7 @@ public final class testInputUrlBar extends BaseTest { } }); mActions.sendKeys("op"); - assertUrlBarText("abopefcghd"); + assertUrlBarText("zyopefcghd"); runOnUiThreadSync(new Runnable() { public void run() { @@ -58,7 +60,7 @@ public final class testInputUrlBar extends BaseTest { } }); mActions.sendKeys("qr"); - assertUrlBarText("abopefqrhd"); + assertUrlBarText("zyopefqrhd"); runOnUiThreadSync(new Runnable() { public void run() { @@ -67,7 +69,7 @@ public final class testInputUrlBar extends BaseTest { } }); mActions.sendKeys("st"); - assertUrlBarText("abstefqrhd"); + assertUrlBarText("zystefqrhd"); runOnUiThreadSync(new Runnable() { public void run() {