diff options
author | mpearson@chromium.org <mpearson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-10 21:08:06 +0000 |
---|---|---|
committer | mpearson@chromium.org <mpearson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-10 21:08:06 +0000 |
commit | 973a886cfda00424c0343fb887da5afa2bec227c (patch) | |
tree | 22994b367e7dce6577829611d4867145361f9a1c | |
parent | d3cb9d234027fdf031a5bc8634469d0103598260 (diff) | |
download | chromium_src-973a886cfda00424c0343fb887da5afa2bec227c.zip chromium_src-973a886cfda00424c0343fb887da5afa2bec227c.tar.gz chromium_src-973a886cfda00424c0343fb887da5afa2bec227c.tar.bz2 |
Omnibox: Make Bookmarks Set Inline_Autocompletion
Make BookmarksProvider set inline_autocompletion and
allowed_to_be_default_match.
This has no effect unless bookmarks are made to score more highly (and
so they'll empirically end up as the top-scoring match).
Includes unit tests.
In the process, fix whether trailing slashes are omitted, for consistency
with other providers. This will reduce jank if bookmarks are
ever inlining. Tested interactively.
Likewise, fix whether http:// is omitted. Tested interactively.
Also, refactor a common piece of code from ShortcutsProvider
to URLPrefix.
Likewise, make a protected function in HistoryProvider public
for use elsewhere. Mark it static (because it can be).
Also, fix a PreventInlineAutocomplete bug in ShortcutsProvider.
Adds a test for this. These tests fail before this change.
Also, fix a PreventInlineAutocomplete bug in BuiltinProvider.
Didn't bother adding a test for this because no tests examine
inline_autocompletion here and I didn't want to bother adding
some just for this minor change. Tested it interactively.
These tests fail before this change.
BUG=
Review URL: https://codereview.chromium.org/229733004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@263077 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/autocomplete/autocomplete_provider.h | 1 | ||||
-rw-r--r-- | chrome/browser/autocomplete/bookmark_provider.cc | 40 | ||||
-rw-r--r-- | chrome/browser/autocomplete/bookmark_provider.h | 11 | ||||
-rw-r--r-- | chrome/browser/autocomplete/bookmark_provider_unittest.cc | 56 | ||||
-rw-r--r-- | chrome/browser/autocomplete/builtin_provider.cc | 4 | ||||
-rw-r--r-- | chrome/browser/autocomplete/history_provider.cc | 28 | ||||
-rw-r--r-- | chrome/browser/autocomplete/history_provider.h | 12 | ||||
-rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider.cc | 88 | ||||
-rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider.h | 15 | ||||
-rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider_unittest.cc | 40 | ||||
-rw-r--r-- | chrome/browser/autocomplete/url_prefix.cc | 54 | ||||
-rw-r--r-- | chrome/browser/autocomplete/url_prefix.h | 17 |
12 files changed, 264 insertions, 102 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_provider.h b/chrome/browser/autocomplete/autocomplete_provider.h index ef7790f..f55e9f6 100644 --- a/chrome/browser/autocomplete/autocomplete_provider.h +++ b/chrome/browser/autocomplete/autocomplete_provider.h @@ -239,6 +239,7 @@ class AutocompleteProvider protected: friend class base::RefCountedThreadSafe<AutocompleteProvider>; + FRIEND_TEST_ALL_PREFIXES(BookmarkProviderTest, InlineAutocompletion); virtual ~AutocompleteProvider(); diff --git a/chrome/browser/autocomplete/bookmark_provider.cc b/chrome/browser/autocomplete/bookmark_provider.cc index 73d1737..fb862fc 100644 --- a/chrome/browser/autocomplete/bookmark_provider.cc +++ b/chrome/browser/autocomplete/bookmark_provider.cc @@ -9,7 +9,10 @@ #include <vector> #include "base/prefs/pref_service.h" +#include "base/strings/utf_string_conversions.h" #include "chrome/browser/autocomplete/autocomplete_result.h" +#include "chrome/browser/autocomplete/history_provider.h" +#include "chrome/browser/autocomplete/url_prefix.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_title_match.h" @@ -39,14 +42,9 @@ void BookmarkProvider::Start(const AutocompleteInput& input, return; matches_.clear(); - // Short-circuit any matching when inline autocompletion is disabled and - // we're looking for BEST_MATCH because none of the BookmarkProvider's - // matches can score high enough to qualify. if (input.text().empty() || ((input.type() != AutocompleteInput::UNKNOWN) && - (input.type() != AutocompleteInput::QUERY)) || - ((input.matches_requested() == AutocompleteInput::BEST_MATCH) && - input.prevent_inline_autocomplete())) + (input.type() != AutocompleteInput::QUERY))) return; DoAutocomplete(input, @@ -94,11 +92,13 @@ void BookmarkProvider::DoAutocomplete(const AutocompleteInput& input, &matches); if (matches.empty()) return; // There were no matches. + AutocompleteInput fixed_up_input(input); + FixupUserInput(&fixed_up_input); for (TitleMatches::const_iterator i = matches.begin(); i != matches.end(); ++i) { // Create and score the AutocompleteMatch. If its score is 0 then the // match is discarded. - AutocompleteMatch match(TitleMatchToACMatch(*i)); + AutocompleteMatch match(TitleMatchToACMatch(input, fixed_up_input, *i)); if (match.relevance > 0) matches_.push_back(match); } @@ -153,6 +153,8 @@ class ScoringFunctor { } // namespace AutocompleteMatch BookmarkProvider::TitleMatchToACMatch( + const AutocompleteInput& input, + const AutocompleteInput& fixed_up_input, const BookmarkTitleMatch& title_match) { // The AutocompleteMatch we construct is non-deletable because the only // way to support this would be to delete the underlying bookmark, which is @@ -161,16 +163,36 @@ AutocompleteMatch BookmarkProvider::TitleMatchToACMatch( AutocompleteMatchType::BOOKMARK_TITLE); const base::string16& title(title_match.node->GetTitle()); DCHECK(!title.empty()); + const GURL& url(title_match.node->url()); + const base::string16& url_utf16 = base::UTF8ToUTF16(url.spec()); + size_t match_start, inline_autocomplete_offset; + URLPrefix::ComputeMatchStartAndInlineAutocompleteOffset( + input, fixed_up_input, false, url_utf16, &match_start, + &inline_autocomplete_offset); match.destination_url = url; + const bool trim_http = !AutocompleteInput::HasHTTPScheme(input.text()) && + ((match_start == base::string16::npos) || (match_start != 0)); match.contents = net::FormatUrl(url, languages_, - net::kFormatUrlOmitAll & net::kFormatUrlOmitHTTP, - net::UnescapeRule::SPACES, NULL, NULL, NULL); + net::kFormatUrlOmitAll & ~(trim_http ? 0 : net::kFormatUrlOmitHTTP), + net::UnescapeRule::SPACES, NULL, NULL, &inline_autocomplete_offset); match.contents_class.push_back( ACMatchClassification(0, ACMatchClassification::URL)); match.fill_into_edit = AutocompleteInput::FormattedStringWithEquivalentMeaning(url, match.contents); + if (inline_autocomplete_offset != base::string16::npos) { + // |inline_autocomplete_offset| may be beyond the end of the + // |fill_into_edit| if the user has typed an URL with a scheme and the + // last character typed is a slash. That slash is removed by the + // FormatURLWithOffsets call above. + if (inline_autocomplete_offset < match.fill_into_edit.length()) { + match.inline_autocompletion = + match.fill_into_edit.substr(inline_autocomplete_offset); + } + match.allowed_to_be_default_match = match.inline_autocompletion.empty() || + !HistoryProvider::PreventInlineAutocomplete(input); + } match.description = title; match.description_class = ClassificationsFromMatch(title_match.match_positions, diff --git a/chrome/browser/autocomplete/bookmark_provider.h b/chrome/browser/autocomplete/bookmark_provider.h index ecd7b70..5ddfaa3 100644 --- a/chrome/browser/autocomplete/bookmark_provider.h +++ b/chrome/browser/autocomplete/bookmark_provider.h @@ -40,6 +40,8 @@ class BookmarkProvider : public AutocompleteProvider { } private: + FRIEND_TEST_ALL_PREFIXES(BookmarkProviderTest, InlineAutocompletion); + virtual ~BookmarkProvider(); // Performs the actual matching of |input| over the bookmarks and fills in @@ -49,8 +51,13 @@ class BookmarkProvider : public AutocompleteProvider { // Compose an AutocompleteMatch based on |title_match| that has 1) the URL of // title_match's bookmark, and 2) the bookmark's title, not the URL's page - // title, as the description. - AutocompleteMatch TitleMatchToACMatch(const BookmarkTitleMatch& title_match); + // title, as the description. |input| is used to compute the match's + // inline_autocompletion. |fixed_up_input| is used in that way as well; + // it's passed separately so this function doesn't have to compute it. + AutocompleteMatch TitleMatchToACMatch( + const AutocompleteInput& input, + const AutocompleteInput& fixed_up_input, + const BookmarkTitleMatch& title_match); // Converts |positions| into ACMatchClassifications and returns the // classifications. |text_length| is used to determine the need to add an diff --git a/chrome/browser/autocomplete/bookmark_provider_unittest.cc b/chrome/browser/autocomplete/bookmark_provider_unittest.cc index 4455cc0..100f0ee 100644 --- a/chrome/browser/autocomplete/bookmark_provider_unittest.cc +++ b/chrome/browser/autocomplete/bookmark_provider_unittest.cc @@ -17,6 +17,7 @@ #include "chrome/browser/autocomplete/autocomplete_provider_listener.h" #include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h" +#include "chrome/browser/bookmarks/bookmark_title_match.h" #include "chrome/test/base/testing_profile.h" #include "testing/gtest/include/gtest/gtest.h" @@ -34,6 +35,12 @@ struct BookmarksTestInfo { { "jkl ghi", "http://www.catsanddogs.com/g" }, { "frankly frankly frank", "http://www.catsanddogs.com/h" }, { "foobar foobar", "http://www.foobar.com/" }, + // For testing inline_autocompletion. + { "http://blah.com/", "http://blah.com/" }, + { "http://fiddle.com/", "http://fiddle.com/" }, + { "http://www.www.com/", "http://www.www.com/" }, + { "chrome://version", "chrome://version" }, + { "chrome://omnibox", "chrome://omnibox" }, // For testing ranking with different URLs. {"achlorhydric featherheads resuscitates mockingbirds", "http://www.featherheads.com/a" }, @@ -339,7 +346,54 @@ TEST_F(BookmarkProviderTest, Rankings) { base::UTF16ToUTF8(matches[j].description)) << " Mismatch at [" << base::IntToString(j) << "] for query '" << query_data[i].query << "'."; - EXPECT_FALSE(matches[j].allowed_to_be_default_match); } } } + +TEST_F(BookmarkProviderTest, InlineAutocompletion) { + // Simulate searches. + struct QueryData { + const std::string query; + const std::string url; + const bool allowed_to_be_default_match; + const std::string inline_autocompletion; + } query_data[] = { + { "bla", "http://blah.com/", true, "h.com" }, + { "blah ", "http://blah.com/", false, ".com" }, + { "http://bl", "http://blah.com/", true, "ah.com" }, + { "fiddle.c", "http://fiddle.com/", true, "om" }, + { "www", "http://www.www.com/", true, ".com" }, + { "chro", "chrome://version", true, "me://version" }, + { "chrome://ve", "chrome://version", true, "rsion" }, + { "chrome ver", "chrome://version", false, "" }, + { "versi", "chrome://version", false, "" }, + { "abou", "chrome://omnibox", false, "" }, + { "about:om", "chrome://omnibox", true, "nibox" } + // Note: when adding a new URL to this test, be sure to add it to the list + // of bookmarks at the top of the file as well. All items in this list + // need to be in the bookmarks list because BookmarkProvider's + // TitleMatchToACMatch() has an assertion that verifies the URL is + // actually bookmarked. + }; + + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(query_data); ++i) { + const std::string description = "for query=" + query_data[i].query + + " and url=" + query_data[i].url; + AutocompleteInput input(base::ASCIIToUTF16(query_data[i].query), + base::string16::npos, base::string16(), GURL(), + AutocompleteInput::INVALID_SPEC, false, false, + false, AutocompleteInput::ALL_MATCHES); + AutocompleteInput fixed_up_input(input); + provider_->FixupUserInput(&fixed_up_input); + BookmarkNode node(GURL(query_data[i].url)); + node.SetTitle(base::ASCIIToUTF16(query_data[i].url)); + BookmarkTitleMatch bookmark_match; + bookmark_match.node = &node; + const AutocompleteMatch& ac_match = + provider_->TitleMatchToACMatch(input, fixed_up_input, bookmark_match); + EXPECT_EQ(query_data[i].allowed_to_be_default_match, + ac_match.allowed_to_be_default_match) << description; + EXPECT_EQ(base::ASCIIToUTF16(query_data[i].inline_autocompletion), + ac_match.inline_autocompletion) << description; + } +} diff --git a/chrome/browser/autocomplete/builtin_provider.cc b/chrome/browser/autocomplete/builtin_provider.cc index 0520281..7e15b39 100644 --- a/chrome/browser/autocomplete/builtin_provider.cc +++ b/chrome/browser/autocomplete/builtin_provider.cc @@ -9,6 +9,7 @@ #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/autocomplete/autocomplete_input.h" +#include "chrome/browser/autocomplete/history_provider.h" #include "chrome/common/net/url_fixer_upper.h" #include "chrome/common/url_constants.h" @@ -119,7 +120,8 @@ void BuiltinProvider::Start(const AutocompleteInput& input, for (size_t i = 0; i < matches_.size(); ++i) matches_[i].relevance = kRelevance + matches_.size() - (i + 1); - if (!input.prevent_inline_autocomplete() && (matches_.size() == 1)) { + if (!HistoryProvider::PreventInlineAutocomplete(input) && + (matches_.size() == 1)) { // If there's only one possible completion of the user's input and // allowing completions is okay, give the match a high enough score to // allow it to beat url-what-you-typed and be inlined. diff --git a/chrome/browser/autocomplete/history_provider.cc b/chrome/browser/autocomplete/history_provider.cc index 989ae38..665f80b 100644 --- a/chrome/browser/autocomplete/history_provider.cc +++ b/chrome/browser/autocomplete/history_provider.cc @@ -18,12 +18,6 @@ #include "chrome/common/url_constants.h" #include "url/url_util.h" -HistoryProvider::HistoryProvider(AutocompleteProviderListener* listener, - Profile* profile, - AutocompleteProvider::Type type) - : AutocompleteProvider(listener, profile, type) { -} - void HistoryProvider::DeleteMatch(const AutocompleteMatch& match) { DCHECK(done_); DCHECK(profile_); @@ -39,6 +33,20 @@ void HistoryProvider::DeleteMatch(const AutocompleteMatch& match) { DeleteMatchFromMatches(match); } +// static +bool HistoryProvider::PreventInlineAutocomplete( + const AutocompleteInput& input) { + return input.prevent_inline_autocomplete() || + (!input.text().empty() && + IsWhitespace(input.text()[input.text().length() - 1])); +} + +HistoryProvider::HistoryProvider(AutocompleteProviderListener* listener, + Profile* profile, + AutocompleteProvider::Type type) + : AutocompleteProvider(listener, profile, type) { +} + HistoryProvider::~HistoryProvider() {} void HistoryProvider::DeleteMatchFromMatches(const AutocompleteMatch& match) { @@ -62,14 +70,6 @@ void HistoryProvider::DeleteMatchFromMatches(const AutocompleteMatch& match) { } // static -bool HistoryProvider::PreventInlineAutocomplete( - const AutocompleteInput& input) { - return input.prevent_inline_autocomplete() || - (!input.text().empty() && - IsWhitespace(input.text()[input.text().length() - 1])); -} - -// static ACMatchClassifications HistoryProvider::SpansFromTermMatch( const history::TermMatches& matches, size_t text_length, diff --git a/chrome/browser/autocomplete/history_provider.h b/chrome/browser/autocomplete/history_provider.h index 9baba60..3e3f837 100644 --- a/chrome/browser/autocomplete/history_provider.h +++ b/chrome/browser/autocomplete/history_provider.h @@ -18,6 +18,11 @@ class HistoryProvider : public AutocompleteProvider { public: virtual void DeleteMatch(const AutocompleteMatch& match) OVERRIDE; + // Returns true if inline autocompletion should be prevented for URL-like + // input. This method returns true if input.prevent_inline_autocomplete() + // is true or the input text contains trailing whitespace. + static bool PreventInlineAutocomplete(const AutocompleteInput& input); + protected: HistoryProvider(AutocompleteProviderListener* listener, Profile* profile, @@ -28,13 +33,6 @@ class HistoryProvider : public AutocompleteProvider { // backing data. void DeleteMatchFromMatches(const AutocompleteMatch& match); - // Returns true if inline autocompletion should be prevented. Use this instead - // of |input.prevent_inline_autocomplete| if the input is passed through - // FixupUserInput(). This method returns true if - // |input.prevent_inline_autocomplete()| is true or the input text contains - // trailing whitespace. - bool PreventInlineAutocomplete(const AutocompleteInput& input); - // Fill and return an ACMatchClassifications structure given the |matches| // to highlight. static ACMatchClassifications SpansFromTermMatch( diff --git a/chrome/browser/autocomplete/shortcuts_provider.cc b/chrome/browser/autocomplete/shortcuts_provider.cc index 8c4d45f..3192ce5 100644 --- a/chrome/browser/autocomplete/shortcuts_provider.cc +++ b/chrome/browser/autocomplete/shortcuts_provider.cc @@ -47,27 +47,6 @@ class DestinationURLEqualsURL { const GURL url_; }; -// Like URLPrefix::BestURLPrefix() except also handles the prefix of -// "www.". This is needed because sometimes the string we're matching -// against here (which comes from |fill_into_edit|) can start with -// "www." without having a protocol at the beginning. Because "www." -// is not on the default prefix list, we test for it explicitly here -// and use that match if the default list didn't have a match or the -// default list's match was shorter than it could've been. -const URLPrefix* BestURLPrefixWithWWWCase( - const base::string16& text, - const base::string16& prefix_suffix) { - CR_DEFINE_STATIC_LOCAL(URLPrefix, www_prefix, - (base::ASCIIToUTF16("www."), 1)); - const URLPrefix* best_prefix = URLPrefix::BestURLPrefix(text, prefix_suffix); - if ((best_prefix == NULL) || - (best_prefix->num_components < www_prefix.num_components)) { - if (URLPrefix::PrefixMatch(www_prefix, text, prefix_suffix)) - best_prefix = &www_prefix; - } - return best_prefix; -} - } // namespace ShortcutsProvider::ShortcutsProvider(AutocompleteProviderListener* listener, @@ -158,12 +137,10 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { base::string16 term_string(base::i18n::ToLower(input.text())); DCHECK(!term_string.empty()); - base::string16 fixed_up_term_string(term_string); AutocompleteInput fixed_up_input(input); - if (FixupUserInput(&fixed_up_input)) - fixed_up_term_string = fixed_up_input.text(); - const GURL& term_string_as_gurl = URLFixerUpper::FixupURL( - base::UTF16ToUTF8(term_string), std::string()); + FixupUserInput(&fixed_up_input); + const GURL& input_as_gurl = URLFixerUpper::FixupURL( + base::UTF16ToUTF8(input.text()), std::string()); int max_relevance; if (!OmniboxFieldTrial::ShortcutsScoringMaxRelevance( @@ -178,8 +155,7 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { int relevance = CalculateScore(term_string, it->second, max_relevance); if (relevance) { matches_.push_back(ShortcutToACMatch( - it->second, relevance, term_string, fixed_up_term_string, - term_string_as_gurl, input.prevent_inline_autocomplete())); + it->second, relevance, input, fixed_up_input, input_as_gurl)); matches_.back().ComputeStrippedDestinationURL(profile_); } } @@ -220,11 +196,10 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { AutocompleteMatch ShortcutsProvider::ShortcutToACMatch( const history::ShortcutsDatabase::Shortcut& shortcut, int relevance, - const base::string16& term_string, - const base::string16& fixed_up_term_string, - const GURL& term_string_as_gurl, - const bool prevent_inline_autocomplete) { - DCHECK(!term_string.empty()); + const AutocompleteInput& input, + const AutocompleteInput& fixed_up_input, + const GURL& input_as_gurl) { + DCHECK(!input.text().empty()); AutocompleteMatch match; match.provider = this; match.relevance = relevance; @@ -251,48 +226,43 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch( // If the match is a search query this is easy: simply check whether the // user text is a prefix of the query. If the match is a navigation, we // assume the fill_into_edit looks something like a URL, so we use - // BestURLPrefix() to try and strip off any prefixes that the user might - // not think would change the meaning, but would otherwise prevent inline - // autocompletion. This allows, for example, the input of "foo.c" to - // autocomplete to "foo.com" for a fill_into_edit of "http://foo.com". + // URLPrefix::ComputeMatchStartAndInlineAutocompleteOffset() to try and strip + // off any prefixes that the user might not think would change the meaning, + // but would otherwise prevent inline autocompletion. This allows, for + // example, the input of "foo.c" to autocomplete to "foo.com" for a + // fill_into_edit of "http://foo.com". if (AutocompleteMatch::IsSearchType(match.type)) { - if (StartsWith(match.fill_into_edit, term_string, false)) { + if (StartsWith(match.fill_into_edit, input.text(), false)) { match.inline_autocompletion = - match.fill_into_edit.substr(term_string.length()); + match.fill_into_edit.substr(input.text().length()); match.allowed_to_be_default_match = - !prevent_inline_autocomplete || match.inline_autocompletion.empty(); + !input.prevent_inline_autocomplete() || + match.inline_autocompletion.empty(); } } else { - const URLPrefix* best_prefix = - BestURLPrefixWithWWWCase(match.fill_into_edit, term_string); - const base::string16* matching_string = &term_string; - // If we failed to find a best_prefix initially, try again using a - // fixed-up version of the user input. This is especially useful to - // get about: URLs to inline against chrome:// shortcuts. (about: - // URLs are fixed up to the chrome:// scheme.) - if ((best_prefix == NULL) && !fixed_up_term_string.empty() && - (fixed_up_term_string != term_string)) { - best_prefix = BestURLPrefixWithWWWCase( - match.fill_into_edit, fixed_up_term_string); - matching_string = &fixed_up_term_string; - } - if (best_prefix != NULL) { - match.inline_autocompletion = match.fill_into_edit.substr( - best_prefix->prefix.length() + matching_string->length()); + size_t match_start, inline_autocomplete_offset; + URLPrefix::ComputeMatchStartAndInlineAutocompleteOffset( + input, fixed_up_input, true, match.fill_into_edit, + &match_start, &inline_autocomplete_offset); + if (inline_autocomplete_offset != base::string16::npos) { + match.inline_autocompletion = + match.fill_into_edit.substr(inline_autocomplete_offset); match.allowed_to_be_default_match = - !prevent_inline_autocomplete || match.inline_autocompletion.empty(); + !HistoryProvider::PreventInlineAutocomplete(input) || + match.inline_autocompletion.empty(); } else { // Also allow a user's input to be marked as default if it would be fixed // up to the same thing as the fill_into_edit. This handles cases like // the user input containing a trailing slash absent in fill_into_edit. - match.allowed_to_be_default_match = (term_string_as_gurl == + match.allowed_to_be_default_match = (input_as_gurl == URLFixerUpper::FixupURL(base::UTF16ToUTF8(match.fill_into_edit), std::string())); } } // Try to mark pieces of the contents and description as matches if they - // appear in |term_string|. + // appear in |input.text()|. + const base::string16 term_string = base::i18n::ToLower(input.text()); WordMap terms_map(CreateWordMapForString(term_string)); if (!terms_map.empty()) { match.contents_class = ClassifyAllMatchesInString(term_string, terms_map, diff --git a/chrome/browser/autocomplete/shortcuts_provider.h b/chrome/browser/autocomplete/shortcuts_provider.h index 5a18f72..93778ec 100644 --- a/chrome/browser/autocomplete/shortcuts_provider.h +++ b/chrome/browser/autocomplete/shortcuts_provider.h @@ -50,18 +50,15 @@ class ShortcutsProvider // Returns an AutocompleteMatch corresponding to |shortcut|. Assigns it // |relevance| score in the process, and highlights the description and - // contents against |term_string|, which should be the lower-cased version - // of the user's input. |term_string|, |fixed_up_term_string|, and - // |term_string_as_gurl| are used to decide what can be inlined. If - // |prevent_inline_autocomplete|, no matches with inline completions will - // be allowed to be the default match. + // contents against |input|, which should be the lower-cased version + // of the user's input. |input|, |fixed_up_input|, and + // |input_as_gurl| are used to decide what can be inlined. AutocompleteMatch ShortcutToACMatch( const history::ShortcutsDatabase::Shortcut& shortcut, int relevance, - const base::string16& term_string, - const base::string16& fixed_up_term_string, - const GURL& term_string_as_gurl, - const bool prevent_inline_autocomplete); + const AutocompleteInput& input, + const AutocompleteInput& fixed_up_input, + const GURL& input_as_gurl); // Returns a map mapping characters to groups of words from |text| that start // with those characters, ordered lexicographically descending so that longer diff --git a/chrome/browser/autocomplete/shortcuts_provider_unittest.cc b/chrome/browser/autocomplete/shortcuts_provider_unittest.cc index 30305d7..511c95f 100644 --- a/chrome/browser/autocomplete/shortcuts_provider_unittest.cc +++ b/chrome/browser/autocomplete/shortcuts_provider_unittest.cc @@ -186,6 +186,16 @@ struct TestShortcutInfo { "0,1", "Foo - Typo in Input Corrected in fill_into_edit", "0,1", content::PAGE_TRANSITION_TYPED, AutocompleteMatchType::HISTORY_URL, "", 1, 100 }, + { "BD85DBA2-8C29-49F9-84AE-48E1E90880FB", "trailing1 ", + "http://trailing1.com", "http://trailing1.com/", + "Trailing1 - Space in Shortcut", "0,1", + "Trailing1 - Space in Shortcut", "0,1", content::PAGE_TRANSITION_TYPED, + AutocompleteMatchType::HISTORY_URL, "", 1, 100 }, + { "BD85DBA2-8C29-49F9-84AE-48E1E90880FC", "about:trailing2 ", + "chrome://trailing2blah", "chrome://trailing2blah/", + "Trailing2 - Space in Shortcut", "0,1", + "Trailing2 - Space in Shortcut", "0,1", content::PAGE_TRANSITION_TYPED, + AutocompleteMatchType::HISTORY_URL, "", 1, 100 }, }; } // namespace @@ -511,6 +521,36 @@ TEST_F(ShortcutsProviderTest, TrickySingleMatch) { expected_urls.push_back( ExpectedURLAndAllowedToBeDefault(expected_url, true)); RunTest(text, true, expected_urls, expected_url, base::string16()); + + // A foursome of tests to verify that trailing spaces prevent the shortcut + // from being allowed to be the default match. For each of two tests, we + // first verify that the match is allowed to be default without the trailing + // space but is not allowed to be default with the trailing space. In both + // of these with-trailing-space cases, we actually get an + // inline_autocompletion, though it's never used because the match is + // prohibited from being default. + text = ASCIIToUTF16("trailing1"); + expected_url = "http://trailing1.com/"; + expected_urls.clear(); + expected_urls.push_back( + ExpectedURLAndAllowedToBeDefault(expected_url, true)); + RunTest(text, false, expected_urls, expected_url, ASCIIToUTF16(".com")); + text = ASCIIToUTF16("trailing1 "); + expected_urls.clear(); + expected_urls.push_back( + ExpectedURLAndAllowedToBeDefault(expected_url, false)); + RunTest(text, false, expected_urls, expected_url, ASCIIToUTF16(".com")); + text = ASCIIToUTF16("about:trailing2"); + expected_url = "chrome://trailing2blah/"; + expected_urls.clear(); + expected_urls.push_back( + ExpectedURLAndAllowedToBeDefault(expected_url, true)); + RunTest(text, false, expected_urls, expected_url, ASCIIToUTF16("blah")); + text = ASCIIToUTF16("about:trailing2 "); + expected_urls.clear(); + expected_urls.push_back( + ExpectedURLAndAllowedToBeDefault(expected_url, false)); + RunTest(text, false, expected_urls, expected_url, ASCIIToUTF16("blah")); } TEST_F(ShortcutsProviderTest, MultiMatch) { diff --git a/chrome/browser/autocomplete/url_prefix.cc b/chrome/browser/autocomplete/url_prefix.cc index b304c7c..1d680db 100644 --- a/chrome/browser/autocomplete/url_prefix.cc +++ b/chrome/browser/autocomplete/url_prefix.cc @@ -7,6 +7,27 @@ #include "base/basictypes.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" +#include "chrome/browser/autocomplete/autocomplete_input.h" + +namespace { + +// Like URLPrefix::BestURLPrefix() except also handles the prefix of +// "www.". +const URLPrefix* BestURLPrefixWithWWWCase( + const base::string16& text, + const base::string16& prefix_suffix) { + CR_DEFINE_STATIC_LOCAL(URLPrefix, www_prefix, + (base::ASCIIToUTF16("www."), 1)); + const URLPrefix* best_prefix = URLPrefix::BestURLPrefix(text, prefix_suffix); + if ((best_prefix == NULL) || + (best_prefix->num_components < www_prefix.num_components)) { + if (URLPrefix::PrefixMatch(www_prefix, text, prefix_suffix)) + best_prefix = &www_prefix; + } + return best_prefix; +} + +} // namespace URLPrefix::URLPrefix(const base::string16& prefix, size_t num_components) : prefix(prefix), @@ -54,3 +75,36 @@ bool URLPrefix::PrefixMatch(const URLPrefix& prefix, const base::string16& prefix_suffix) { return StartsWith(text, prefix.prefix + prefix_suffix, false); } + +// static +void URLPrefix::ComputeMatchStartAndInlineAutocompleteOffset( + const AutocompleteInput& input, + const AutocompleteInput& fixed_up_input, + const bool allow_www_prefix_without_scheme, + const base::string16& text, + size_t* match_start, + size_t* inline_autocomplete_offset) { + const URLPrefix* best_prefix = allow_www_prefix_without_scheme ? + BestURLPrefixWithWWWCase(text, input.text()) : + BestURLPrefix(text, input.text()); + const base::string16* matching_string = &input.text(); + // If we failed to find a best_prefix initially, try again using a fixed-up + // version of the user input. This is especially useful to get about: URLs + // to inline against chrome:// shortcuts. (about: URLs are fixed up to the + // chrome:// scheme.) + if ((best_prefix == NULL) && !fixed_up_input.text().empty() && + (fixed_up_input.text() != input.text())) { + best_prefix = allow_www_prefix_without_scheme ? + BestURLPrefixWithWWWCase(text, fixed_up_input.text()) : + BestURLPrefix(text, fixed_up_input.text()); + matching_string = &fixed_up_input.text(); + } + if (best_prefix != NULL) { + *match_start = best_prefix->prefix.length(); + *inline_autocomplete_offset = + best_prefix->prefix.length() + matching_string->length(); + } else { + *match_start = base::string16::npos; + *inline_autocomplete_offset = base::string16::npos; + } +} diff --git a/chrome/browser/autocomplete/url_prefix.h b/chrome/browser/autocomplete/url_prefix.h index e3d1894..f5494cb 100644 --- a/chrome/browser/autocomplete/url_prefix.h +++ b/chrome/browser/autocomplete/url_prefix.h @@ -8,6 +8,7 @@ #include <vector> #include "base/strings/string16.h" +#include "chrome/browser/autocomplete/autocomplete_input.h" struct URLPrefix; typedef std::vector<URLPrefix> URLPrefixes; @@ -37,6 +38,22 @@ struct URLPrefix { const base::string16& text, const base::string16& prefix_suffix); + // Sees if |text| is inlineable against either |input| or |fixed_up_input|, + // filling in |match_start| and |inline_autocomplete_offset| appropriately. + // |allow_www_prefix_without_scheme| says whether to consider an input such + // as "foo" to be allowed to match against text "www.foo.com". This is + // needed because sometimes the string we're matching against here can come + // from a match's fill_into_edit, which can start with "www." without having + // a protocol at the beginning, and we want to allow these matches to be + // inlineable. ("www." is not otherwise on the default prefix list.) + static void ComputeMatchStartAndInlineAutocompleteOffset( + const AutocompleteInput& input, + const AutocompleteInput& fixed_up_input, + const bool allow_www_prefix_without_scheme, + const base::string16& text, + size_t* match_start, + size_t* inline_autocomplete_offset); + base::string16 prefix; // The number of URL components (scheme, domain label, etc.) in the prefix. |