summaryrefslogtreecommitdiffstats
path: root/chrome/browser/autocomplete
diff options
context:
space:
mode:
authorpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-11-03 18:19:37 +0000
committerpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-11-03 18:19:37 +0000
commita08e630e67ce28cd084841b1b11dd79a6daa91a9 (patch)
treeb484e2ab143eca4fd5482965cebec3da2d7a8bf7 /chrome/browser/autocomplete
parent4a6621d76d5c1f12746a06cb8efd96c1224ab9c3 (diff)
downloadchromium_src-a08e630e67ce28cd084841b1b11dd79a6daa91a9.zip
chromium_src-a08e630e67ce28cd084841b1b11dd79a6daa91a9.tar.gz
chromium_src-a08e630e67ce28cd084841b1b11dd79a6daa91a9.tar.bz2
Better handling of UNKNOWN versus QUERY in a couple ways.
First, made it possible to navigate to "invalid" hostnames in a number of additional cases, including when the user types an explicit scheme or when we have a valid TLD and no spaces. The default action for these kinds of inputs is still search, but we'll pull up an accidental search infobar if need be now. Second, made the HistoryURLProvider show a What You Typed match for UNKNOWN inputs when the user explicitly typed "http:" or "https:" at the front, since these have the highest probability of being navigational inputs. BUG=26341 TEST=Type "http://foo bar baz.com" and verify you are given a (non-default) entry in the dropdown to navigate. Review URL: http://codereview.chromium.org/353010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30830 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/autocomplete')
-rw-r--r--chrome/browser/autocomplete/autocomplete.cc25
-rw-r--r--chrome/browser/autocomplete/autocomplete_unittest.cc11
-rw-r--r--chrome/browser/autocomplete/history_url_provider.cc21
-rw-r--r--chrome/browser/autocomplete/history_url_provider_unittest.cc27
4 files changed, 69 insertions, 15 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index db663d3..2e7c7ec 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -182,8 +182,29 @@ AutocompleteInput::Type AutocompleteInput::Parse(
url_canon::CanonHostInfo host_info;
const std::string canonicalized_host(net::CanonicalizeHost(host, &host_info));
if ((host_info.family == url_canon::CanonHostInfo::NEUTRAL) &&
- !net::IsCanonicalizedHostCompliant(canonicalized_host))
- return QUERY;
+ !net::IsCanonicalizedHostCompliant(canonicalized_host)) {
+ // Invalid hostname. There are several possible cases:
+ // * Our checker is too strict and the user pasted in a real-world URL
+ // that's "invalid" but resolves. To catch these, we return UNKNOWN when
+ // the user explicitly typed a scheme, so we'll still search by default
+ // but we'll show the accidental search infobar if necessary.
+ // * The user is typing a multi-word query. If we see a space anywhere in
+ // the hostname we assume this is a search and return QUERY.
+ // * Our checker is too strict and the user is typing a real-world hostname
+ // that's "invalid" but resolves. We return UNKNOWN if the TLD is known.
+ // Note that we explicitly excluded hosts with spaces above so that
+ // "toys at amazon.com" will be treated as a search.
+ // * The user is typing some garbage string. Return QUERY.
+ //
+ // Thus we fall down in the following cases:
+ // * Trying to navigate to a hostname with spaces
+ // * Trying to navigate to a hostname with invalid characters and an unknown
+ // TLD
+ // These are rare, though probably possible in intranets.
+ return (parts->scheme.is_nonempty() ||
+ ((registry_length != 0) && (host.find(' ') == std::wstring::npos))) ?
+ UNKNOWN : QUERY;
+ }
// Presence of a port means this is likely a URL, if the port is really a port
// number. If it's just garbage after a colon, this is a query.
diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc
index 45a9fe7..adff59c 100644
--- a/chrome/browser/autocomplete/autocomplete_unittest.cc
+++ b/chrome/browser/autocomplete/autocomplete_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -212,11 +212,14 @@ TEST(AutocompleteTest, InputType) {
{ L"foo", AutocompleteInput::UNKNOWN },
{ L"foo.c", AutocompleteInput::UNKNOWN },
{ L"foo.com", AutocompleteInput::URL },
- { L"-.com", AutocompleteInput::QUERY },
+ { L"-.com", AutocompleteInput::UNKNOWN },
{ L"foo/bar", AutocompleteInput::URL },
{ L"foo/bar baz", AutocompleteInput::UNKNOWN },
+ { L"foo bar.com", AutocompleteInput::QUERY },
{ L"http://foo/bar baz", AutocompleteInput::URL },
{ L"foo bar", AutocompleteInput::QUERY },
+ { L"foo+bar", AutocompleteInput::QUERY },
+ { L"foo+bar.com", AutocompleteInput::UNKNOWN },
{ L"\"foo:bar\"", AutocompleteInput::QUERY },
{ L"link:foo.com", AutocompleteInput::UNKNOWN },
{ L"www.foo.com:81", AutocompleteInput::URL },
@@ -244,8 +247,8 @@ TEST(AutocompleteTest, InputType) {
{ L"http://foo.c", AutocompleteInput::URL },
{ L"http://foo.com", AutocompleteInput::URL },
{ L"http://foo_bar.com", AutocompleteInput::URL },
- { L"http://-.com", AutocompleteInput::QUERY },
- { L"http://_foo_.com", AutocompleteInput::QUERY },
+ { L"http://-.com", AutocompleteInput::UNKNOWN },
+ { L"http://_foo_.com", AutocompleteInput::UNKNOWN },
{ L"http://foo.com:abc", AutocompleteInput::QUERY },
{ L"http://foo.com:123456", AutocompleteInput::QUERY },
{ L"http:user@foo.com", AutocompleteInput::URL },
diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index a266201..b44b6e7 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -127,10 +127,21 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
history::URLDatabase* db,
HistoryURLProviderParams* params) {
// Create a What You Typed match, which we'll need below.
+ //
+ // We display this to the user when there's a reasonable chance they actually
+ // care:
+ // * Their input can be opened as a URL, and
+ // * They hit ctrl-enter, or we parsed the input as a URL, or it starts with
+ // an explicit "http:" or "https:".
+ // Otherwise, this is just low-quality noise. In the cases where we've parsed
+ // as UNKNOWN, we'll still show an accidental search infobar if need be.
bool have_what_you_typed_match =
params->input.canonicalized_url().is_valid() &&
- (params->input.type() != AutocompleteInput::UNKNOWN) &&
- (params->input.type() != AutocompleteInput::QUERY);
+ (params->input.type() != AutocompleteInput::QUERY) &&
+ ((params->input.type() != AutocompleteInput::UNKNOWN) ||
+ !params->trim_http ||
+ url_util::FindAndCompareScheme(WideToUTF8(params->input.text()),
+ chrome::kHttpsScheme, NULL));
AutocompleteMatch what_you_typed_match(SuggestExactInput(params->input,
params->trim_http));
@@ -141,7 +152,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
for (Prefixes::const_iterator i(prefixes_.begin()); i != prefixes_.end();
++i) {
if (params->cancel)
- return; // canceled in the middle of a query, give up
+ return; // Canceled in the middle of a query, give up.
// We only need max_matches results in the end, but before we get there we
// need to promote lower-quality matches that are prefixes of
// higher-quality matches, and remove lower-quality redirects. So we ask
@@ -184,7 +195,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
history_matches.empty() ||
!PromoteMatchForInlineAutocomplete(params, history_matches.front())) {
// Failed to promote any URLs for inline autocompletion. Use the What You
- // Typed match, if we have it and the input looked like a URL.
+ // Typed match, if we have it.
first_match = 0;
if (have_what_you_typed_match)
params->matches.push_back(what_you_typed_match);
diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc
index 49c69a4..408526a 100644
--- a/chrome/browser/autocomplete/history_url_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc
@@ -184,7 +184,7 @@ void HistoryURLProviderTest::RunTest(const std::wstring text,
MessageLoop::current()->Run();
matches_ = autocomplete_->matches();
- ASSERT_EQ(num_results, matches_.size());
+ ASSERT_EQ(num_results, matches_.size()) << "Input text: " << text;
for (size_t i = 0; i < num_results; ++i)
EXPECT_EQ(expected_urls[i], matches_[i].destination_url.spec());
}
@@ -320,6 +320,25 @@ TEST_F(HistoryURLProviderTest, CullRedirects) {
arraysize(expected_results));
}
+TEST_F(HistoryURLProviderTest, WhatYouTyped) {
+ // Make sure we suggest a What You Typed match at the right times.
+ RunTest(L"wytmatch", std::wstring(), false, NULL, 0);
+ RunTest(L"wytmatch foo bar", std::wstring(), false, NULL, 0);
+ RunTest(L"wytmatch+foo+bar", std::wstring(), false, NULL, 0);
+ RunTest(L"wytmatch+foo+bar.com", std::wstring(), false, NULL, 0);
+
+ const std::string results_1[] = {"http://www.wytmatch.com/"};
+ RunTest(L"wytmatch", L"com", false, results_1, arraysize(results_1));
+
+ const std::string results_2[] = {"http://wytmatch%20foo%20bar/"};
+ RunTest(L"http://wytmatch foo bar", std::wstring(), false, results_2,
+ arraysize(results_2));
+
+ const std::string results_3[] = {"https://wytmatch%20foo%20bar/"};
+ RunTest(L"https://wytmatch foo bar", std::wstring(), false, results_3,
+ arraysize(results_3));
+}
+
TEST_F(HistoryURLProviderTest, Fixup) {
// Test for various past crashes we've had.
RunTest(L"\\", std::wstring(), false, NULL, 0);
@@ -350,16 +369,16 @@ TEST_F(HistoryURLProviderTest, Fixup) {
// Adding a TLD to a small number like "56" should result in "www.56.com"
// rather than "0.0.0.56.com".
- std::string fixup_3[] = {"http://www.56.com/"};
+ const std::string fixup_3[] = {"http://www.56.com/"};
RunTest(L"56", L"com", true, fixup_3, arraysize(fixup_3));
// An input looks like a IP address like "127.0.0.1" should result in
// "http://127.0.0.1/".
- std::string fixup_4[] = {"http://127.0.0.1/"};
+ const std::string fixup_4[] = {"http://127.0.0.1/"};
RunTest(L"127.0.0.1", std::wstring(), false, fixup_4, arraysize(fixup_4));
// An number "17173" should result in "http://www.17173.com/" in db.
- std::string fixup_5[] = {"http://www.17173.com/"};
+ const std::string fixup_5[] = {"http://www.17173.com/"};
RunTest(L"17173", std::wstring(), false, fixup_5, arraysize(fixup_5));
}