Commit 973a886c authored by mpearson@chromium.org's avatar mpearson@chromium.org

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
parent d3cb9d23
...@@ -239,6 +239,7 @@ class AutocompleteProvider ...@@ -239,6 +239,7 @@ class AutocompleteProvider
protected: protected:
friend class base::RefCountedThreadSafe<AutocompleteProvider>; friend class base::RefCountedThreadSafe<AutocompleteProvider>;
FRIEND_TEST_ALL_PREFIXES(BookmarkProviderTest, InlineAutocompletion);
virtual ~AutocompleteProvider(); virtual ~AutocompleteProvider();
......
...@@ -9,7 +9,10 @@ ...@@ -9,7 +9,10 @@
#include <vector> #include <vector>
#include "base/prefs/pref_service.h" #include "base/prefs/pref_service.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/autocomplete/autocomplete_result.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.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h" #include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/bookmarks/bookmark_title_match.h" #include "chrome/browser/bookmarks/bookmark_title_match.h"
...@@ -39,14 +42,9 @@ void BookmarkProvider::Start(const AutocompleteInput& input, ...@@ -39,14 +42,9 @@ void BookmarkProvider::Start(const AutocompleteInput& input,
return; return;
matches_.clear(); 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() || if (input.text().empty() ||
((input.type() != AutocompleteInput::UNKNOWN) && ((input.type() != AutocompleteInput::UNKNOWN) &&
(input.type() != AutocompleteInput::QUERY)) || (input.type() != AutocompleteInput::QUERY)))
((input.matches_requested() == AutocompleteInput::BEST_MATCH) &&
input.prevent_inline_autocomplete()))
return; return;
DoAutocomplete(input, DoAutocomplete(input,
...@@ -94,11 +92,13 @@ void BookmarkProvider::DoAutocomplete(const AutocompleteInput& input, ...@@ -94,11 +92,13 @@ void BookmarkProvider::DoAutocomplete(const AutocompleteInput& input,
&matches); &matches);
if (matches.empty()) if (matches.empty())
return; // There were no matches. return; // There were no matches.
AutocompleteInput fixed_up_input(input);
FixupUserInput(&fixed_up_input);
for (TitleMatches::const_iterator i = matches.begin(); i != matches.end(); for (TitleMatches::const_iterator i = matches.begin(); i != matches.end();
++i) { ++i) {
// Create and score the AutocompleteMatch. If its score is 0 then the // Create and score the AutocompleteMatch. If its score is 0 then the
// match is discarded. // match is discarded.
AutocompleteMatch match(TitleMatchToACMatch(*i)); AutocompleteMatch match(TitleMatchToACMatch(input, fixed_up_input, *i));
if (match.relevance > 0) if (match.relevance > 0)
matches_.push_back(match); matches_.push_back(match);
} }
...@@ -153,6 +153,8 @@ class ScoringFunctor { ...@@ -153,6 +153,8 @@ class ScoringFunctor {
} // namespace } // namespace
AutocompleteMatch BookmarkProvider::TitleMatchToACMatch( AutocompleteMatch BookmarkProvider::TitleMatchToACMatch(
const AutocompleteInput& input,
const AutocompleteInput& fixed_up_input,
const BookmarkTitleMatch& title_match) { const BookmarkTitleMatch& title_match) {
// The AutocompleteMatch we construct is non-deletable because the only // The AutocompleteMatch we construct is non-deletable because the only
// way to support this would be to delete the underlying bookmark, which is // way to support this would be to delete the underlying bookmark, which is
...@@ -161,16 +163,36 @@ AutocompleteMatch BookmarkProvider::TitleMatchToACMatch( ...@@ -161,16 +163,36 @@ AutocompleteMatch BookmarkProvider::TitleMatchToACMatch(
AutocompleteMatchType::BOOKMARK_TITLE); AutocompleteMatchType::BOOKMARK_TITLE);
const base::string16& title(title_match.node->GetTitle()); const base::string16& title(title_match.node->GetTitle());
DCHECK(!title.empty()); DCHECK(!title.empty());
const GURL& url(title_match.node->url()); 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; 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_, match.contents = net::FormatUrl(url, languages_,
net::kFormatUrlOmitAll & net::kFormatUrlOmitHTTP, net::kFormatUrlOmitAll & ~(trim_http ? 0 : net::kFormatUrlOmitHTTP),
net::UnescapeRule::SPACES, NULL, NULL, NULL); net::UnescapeRule::SPACES, NULL, NULL, &inline_autocomplete_offset);
match.contents_class.push_back( match.contents_class.push_back(
ACMatchClassification(0, ACMatchClassification::URL)); ACMatchClassification(0, ACMatchClassification::URL));
match.fill_into_edit = match.fill_into_edit =
AutocompleteInput::FormattedStringWithEquivalentMeaning(url, AutocompleteInput::FormattedStringWithEquivalentMeaning(url,
match.contents); 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 = title;
match.description_class = match.description_class =
ClassificationsFromMatch(title_match.match_positions, ClassificationsFromMatch(title_match.match_positions,
......
...@@ -40,6 +40,8 @@ class BookmarkProvider : public AutocompleteProvider { ...@@ -40,6 +40,8 @@ class BookmarkProvider : public AutocompleteProvider {
} }
private: private:
FRIEND_TEST_ALL_PREFIXES(BookmarkProviderTest, InlineAutocompletion);
virtual ~BookmarkProvider(); virtual ~BookmarkProvider();
// Performs the actual matching of |input| over the bookmarks and fills in // Performs the actual matching of |input| over the bookmarks and fills in
...@@ -49,8 +51,13 @@ class BookmarkProvider : public AutocompleteProvider { ...@@ -49,8 +51,13 @@ class BookmarkProvider : public AutocompleteProvider {
// Compose an AutocompleteMatch based on |title_match| that has 1) the URL of // 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_match's bookmark, and 2) the bookmark's title, not the URL's page
// title, as the description. // title, as the description. |input| is used to compute the match's
AutocompleteMatch TitleMatchToACMatch(const BookmarkTitleMatch& title_match); // 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 // Converts |positions| into ACMatchClassifications and returns the
// classifications. |text_length| is used to determine the need to add an // classifications. |text_length| is used to determine the need to add an
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "chrome/browser/autocomplete/autocomplete_provider_listener.h" #include "chrome/browser/autocomplete/autocomplete_provider_listener.h"
#include "chrome/browser/bookmarks/bookmark_model.h" #include "chrome/browser/bookmarks/bookmark_model.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.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 "chrome/test/base/testing_profile.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -34,6 +35,12 @@ struct BookmarksTestInfo { ...@@ -34,6 +35,12 @@ struct BookmarksTestInfo {
{ "jkl ghi", "http://www.catsanddogs.com/g" }, { "jkl ghi", "http://www.catsanddogs.com/g" },
{ "frankly frankly frank", "http://www.catsanddogs.com/h" }, { "frankly frankly frank", "http://www.catsanddogs.com/h" },
{ "foobar foobar", "http://www.foobar.com/" }, { "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. // For testing ranking with different URLs.
{"achlorhydric featherheads resuscitates mockingbirds", {"achlorhydric featherheads resuscitates mockingbirds",
"http://www.featherheads.com/a" }, "http://www.featherheads.com/a" },
...@@ -339,7 +346,54 @@ TEST_F(BookmarkProviderTest, Rankings) { ...@@ -339,7 +346,54 @@ TEST_F(BookmarkProviderTest, Rankings) {
base::UTF16ToUTF8(matches[j].description)) base::UTF16ToUTF8(matches[j].description))
<< " Mismatch at [" << base::IntToString(j) << "] for query '" << " Mismatch at [" << base::IntToString(j) << "] for query '"
<< query_data[i].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;
}
}
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/autocomplete/autocomplete_input.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/net/url_fixer_upper.h"
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
...@@ -119,7 +120,8 @@ void BuiltinProvider::Start(const AutocompleteInput& input, ...@@ -119,7 +120,8 @@ void BuiltinProvider::Start(const AutocompleteInput& input,
for (size_t i = 0; i < matches_.size(); ++i) for (size_t i = 0; i < matches_.size(); ++i)
matches_[i].relevance = kRelevance + matches_.size() - (i + 1); 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 // 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 // allowing completions is okay, give the match a high enough score to
// allow it to beat url-what-you-typed and be inlined. // allow it to beat url-what-you-typed and be inlined.
......
...@@ -18,12 +18,6 @@ ...@@ -18,12 +18,6 @@
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "url/url_util.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) { void HistoryProvider::DeleteMatch(const AutocompleteMatch& match) {
DCHECK(done_); DCHECK(done_);
DCHECK(profile_); DCHECK(profile_);
...@@ -39,6 +33,20 @@ void HistoryProvider::DeleteMatch(const AutocompleteMatch& match) { ...@@ -39,6 +33,20 @@ void HistoryProvider::DeleteMatch(const AutocompleteMatch& match) {
DeleteMatchFromMatches(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() {} HistoryProvider::~HistoryProvider() {}
void HistoryProvider::DeleteMatchFromMatches(const AutocompleteMatch& match) { void HistoryProvider::DeleteMatchFromMatches(const AutocompleteMatch& match) {
...@@ -61,14 +69,6 @@ void HistoryProvider::DeleteMatchFromMatches(const AutocompleteMatch& match) { ...@@ -61,14 +69,6 @@ void HistoryProvider::DeleteMatchFromMatches(const AutocompleteMatch& match) {
DCHECK(found) << "Asked to delete a URL that isn't in our set of matches"; DCHECK(found) << "Asked to delete a URL that isn't in our set of matches";
} }
// static
bool HistoryProvider::PreventInlineAutocomplete(
const AutocompleteInput& input) {
return input.prevent_inline_autocomplete() ||
(!input.text().empty() &&
IsWhitespace(input.text()[input.text().length() - 1]));
}
// static // static
ACMatchClassifications HistoryProvider::SpansFromTermMatch( ACMatchClassifications HistoryProvider::SpansFromTermMatch(
const history::TermMatches& matches, const history::TermMatches& matches,
......
...@@ -18,6 +18,11 @@ class HistoryProvider : public AutocompleteProvider { ...@@ -18,6 +18,11 @@ class HistoryProvider : public AutocompleteProvider {
public: public:
virtual void DeleteMatch(const AutocompleteMatch& match) OVERRIDE; 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: protected:
HistoryProvider(AutocompleteProviderListener* listener, HistoryProvider(AutocompleteProviderListener* listener,
Profile* profile, Profile* profile,
...@@ -28,13 +33,6 @@ class HistoryProvider : public AutocompleteProvider { ...@@ -28,13 +33,6 @@ class HistoryProvider : public AutocompleteProvider {
// backing data. // backing data.
void DeleteMatchFromMatches(const AutocompleteMatch& match); 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| // Fill and return an ACMatchClassifications structure given the |matches|
// to highlight. // to highlight.
static ACMatchClassifications SpansFromTermMatch( static ACMatchClassifications SpansFromTermMatch(
......
...@@ -47,27 +47,6 @@ class DestinationURLEqualsURL { ...@@ -47,27 +47,6 @@ class DestinationURLEqualsURL {
const GURL url_; 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 } // namespace
ShortcutsProvider::ShortcutsProvider(AutocompleteProviderListener* listener, ShortcutsProvider::ShortcutsProvider(AutocompleteProviderListener* listener,
...@@ -158,12 +137,10 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { ...@@ -158,12 +137,10 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) {
base::string16 term_string(base::i18n::ToLower(input.text())); base::string16 term_string(base::i18n::ToLower(input.text()));
DCHECK(!term_string.empty()); DCHECK(!term_string.empty());
base::string16 fixed_up_term_string(term_string);
AutocompleteInput fixed_up_input(input); AutocompleteInput fixed_up_input(input);
if (FixupUserInput(&fixed_up_input)) FixupUserInput(&fixed_up_input);
fixed_up_term_string = fixed_up_input.text(); const GURL& input_as_gurl = URLFixerUpper::FixupURL(
const GURL& term_string_as_gurl = URLFixerUpper::FixupURL( base::UTF16ToUTF8(input.text()), std::string());
base::UTF16ToUTF8(term_string), std::string());
int max_relevance; int max_relevance;
if (!OmniboxFieldTrial::ShortcutsScoringMaxRelevance( if (!OmniboxFieldTrial::ShortcutsScoringMaxRelevance(
...@@ -178,8 +155,7 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { ...@@ -178,8 +155,7 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) {
int relevance = CalculateScore(term_string, it->second, max_relevance); int relevance = CalculateScore(term_string, it->second, max_relevance);
if (relevance) { if (relevance) {
matches_.push_back(ShortcutToACMatch( matches_.push_back(ShortcutToACMatch(
it->second, relevance, term_string, fixed_up_term_string, it->second, relevance, input, fixed_up_input, input_as_gurl));
term_string_as_gurl, input.prevent_inline_autocomplete()));
matches_.back().ComputeStrippedDestinationURL(profile_); matches_.back().ComputeStrippedDestinationURL(profile_);
} }
} }
...@@ -220,11 +196,10 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { ...@@ -220,11 +196,10 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) {
AutocompleteMatch ShortcutsProvider::ShortcutToACMatch( AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
const history::ShortcutsDatabase::Shortcut& shortcut, const history::ShortcutsDatabase::Shortcut& shortcut,
int relevance, int relevance,
const base::string16& term_string, const AutocompleteInput& input,
const base::string16& fixed_up_term_string, const AutocompleteInput& fixed_up_input,
const GURL& term_string_as_gurl, const GURL& input_as_gurl) {
const bool prevent_inline_autocomplete) { DCHECK(!input.text().empty());
DCHECK(!term_string.empty());
AutocompleteMatch match; AutocompleteMatch match;
match.provider = this; match.provider = this;
match.relevance = relevance; match.relevance = relevance;
...@@ -251,48 +226,43 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch( ...@@ -251,48 +226,43 @@ AutocompleteMatch ShortcutsProvider::ShortcutToACMatch(
// If the match is a search query this is easy: simply check whether the // 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 // 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 // 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 // URLPrefix::ComputeMatchStartAndInlineAutocompleteOffset() to try and strip
// not think would change the meaning, but would otherwise prevent inline // off any prefixes that the user might not think would change the meaning,
// autocompletion. This allows, for example, the input of "foo.c" to // but would otherwise prevent inline autocompletion. This allows, for
// autocomplete to "foo.com" for a fill_into_edit of "http://foo.com". // 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 (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.inline_autocompletion =
match.fill_into_edit.substr(term_string.length()); match.fill_into_edit.substr(input.text().length());
match.allowed_to_be_default_match = match.allowed_to_be_default_match =
!prevent_inline_autocomplete || match.inline_autocompletion.empty(); !input.prevent_inline_autocomplete() ||
match.inline_autocompletion.empty();
} }
} else { } else {
const URLPrefix* best_prefix = size_t match_start, inline_autocomplete_offset;
BestURLPrefixWithWWWCase(match.fill_into_edit, term_string); URLPrefix::ComputeMatchStartAndInlineAutocompleteOffset(
const base::string16* matching_string = &term_string; input, fixed_up_input, true, match.fill_into_edit,
// If we failed to find a best_prefix initially, try again using a &match_start, &inline_autocomplete_offset);
// fixed-up version of the user input. This is especially useful to if (inline_autocomplete_offset != base::string16::npos) {
// get about: URLs to inline against chrome:// shortcuts. (about: match.inline_autocompletion =
// URLs are fixed up to the chrome:// scheme.) match.fill_into_edit.substr(inline_autocomplete_offset);
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());
match.allowed_to_be_default_match = match.allowed_to_be_default_match =
!prevent_inline_autocomplete || match.inline_autocompletion.empty(); !HistoryProvider::PreventInlineAutocomplete(input) ||
match.inline_autocompletion.empty();
} else { } else {
// Also allow a user's input to be marked as default if it would be fixed // 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 // 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. // 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), URLFixerUpper::FixupURL(base::UTF16ToUTF8(match.fill_into_edit),
std::string())); std::string()));
} }
} }
// Try to mark pieces of the contents and description as matches if they // 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)); WordMap terms_map(CreateWordMapForString(term_string));
if (!terms_map.empty()) { if (!terms_map.empty()) {
match.contents_class = ClassifyAllMatchesInString(term_string, terms_map, match.contents_class = ClassifyAllMatchesInString(term_string, terms_map,
......
...@@ -50,18 +50,15 @@ class ShortcutsProvider ...@@ -50,18 +50,15 @@ class ShortcutsProvider
// Returns an AutocompleteMatch corresponding to |shortcut|. Assigns it // Returns an AutocompleteMatch corresponding to |shortcut|. Assigns it
// |relevance| score in the process, and highlights the description and // |relevance| score in the process, and highlights the description and
// contents against |term_string|, which should be the lower-cased version // contents against |input|, which should be the lower-cased version
// of the user's input. |term_string|, |fixed_up_term_string|, and // of the user's input. |input|, |fixed_up_input|, and
// |term_string_as_gurl| are used to decide what can be inlined. If // |input_as_gurl| are used to decide what can be inlined.
// |prevent_inline_autocomplete|, no matches with inline completions will
// be allowed to be the default match.
AutocompleteMatch ShortcutToACMatch( AutocompleteMatch ShortcutToACMatch(
const history::ShortcutsDatabase::Shortcut& shortcut, const history::ShortcutsDatabase::Shortcut& shortcut,
int relevance, int relevance,
const base::string16& term_string, const AutocompleteInput& input,
const base::string16& fixed_up_term_string, const AutocompleteInput& fixed_up_input,
const GURL& term_string_as_gurl, const GURL& input_as_gurl);
const bool prevent_inline_autocomplete);
// Returns a map mapping characters to groups of words from |text| that start // Returns a map mapping characters to groups of words from |text| that start
// with those characters, ordered lexicographically descending so that longer // with those characters, ordered lexicographically descending so that longer
......
...@@ -186,6 +186,16 @@ struct TestShortcutInfo { ...@@ -186,6 +186,16 @@ struct TestShortcutInfo {
"0,1", "Foo - Typo in Input Corrected in fill_into_edit", "0,1", "0,1", "Foo - Typo in Input Corrected in fill_into_edit", "0,1",
content::PAGE_TRANSITION_TYPED, AutocompleteMatchType::HISTORY_URL, "", content::PAGE_TRANSITION_TYPED, AutocompleteMatchType::HISTORY_URL, "",
1, 100 }, 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 } // namespace
...@@ -511,6 +521,36 @@ TEST_F(ShortcutsProviderTest, TrickySingleMatch) { ...@@ -511,6 +521,36 @@ TEST_F(ShortcutsProviderTest, TrickySingleMatch) {
expected_urls.push_back( expected_urls.push_back(
ExpectedURLAndAllowedToBeDefault(expected_url, true)); ExpectedURLAndAllowedToBeDefault(expected_url, true));
RunTest(text, true, expected_urls, expected_url, base::string16()); 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) { TEST_F(ShortcutsProviderTest, MultiMatch) {
......
...@@ -7,6 +7,27 @@ ...@@ -7,6 +7,27 @@
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.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) URLPrefix::URLPrefix(const base::string16& prefix, size_t num_components)
: prefix(prefix), : prefix(prefix),
...@@ -54,3 +75,36 @@ bool URLPrefix::PrefixMatch(const URLPrefix& prefix, ...@@ -54,3 +75,36 @@ bool URLPrefix::PrefixMatch(const URLPrefix& prefix,
const base::string16& prefix_suffix) { const base::string16& prefix_suffix) {
return StartsWith(text, prefix.prefix + prefix_suffix, false); 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;
}
}
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <vector> #include <vector>
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "chrome/browser/autocomplete/autocomplete_input.h"
struct URLPrefix; struct URLPrefix;
typedef std::vector<URLPrefix> URLPrefixes; typedef std::vector<URLPrefix> URLPrefixes;
...@@ -37,6 +38,22 @@ struct URLPrefix { ...@@ -37,6 +38,22 @@ struct URLPrefix {
const base::string16& text, const base::string16& text,
const base::string16& prefix_suffix); 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; base::string16 prefix;
// The number of URL components (scheme, domain label, etc.) in the prefix. // The number of URL components (scheme, domain label, etc.) in the prefix.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment