diff options
Diffstat (limited to 'chrome')
5 files changed, 67 insertions, 12 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index e15a190..d45c4a0 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -903,10 +903,8 @@ void AutocompleteController::CommitIfQueryHasNeverBeenCommitted() { void AutocompleteController::OnProviderUpdate(bool updated_matches) { CheckIfDone(); - if (updated_matches || done_) { + if (updated_matches || done_) UpdateLatestResult(false); - return; - } } void AutocompleteController::UpdateLatestResult(bool is_synchronous_pass) { diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc index dc4fde3..230cbf7 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.cc +++ b/chrome/browser/autocomplete/autocomplete_edit.cc @@ -170,7 +170,20 @@ void AutocompleteEditModel::GetDataForURLExport(GURL* url, } std::wstring AutocompleteEditModel::GetDesiredTLD() const { - return (control_key_state_ == DOWN_WITHOUT_CHANGE) ? + // Tricky corner case: The user has typed "foo" and currently sees an inline + // autocomplete suggestion of "foo.net". He now presses ctrl-a (e.g. to + // select all, on Windows). If we treat the ctrl press as potentially for the + // sake of ctrl-enter, then we risk "www.foo.com" being promoted as the best + // match. This would make the autocompleted text disappear, leaving our user + // feeling very confused when the wrong text gets highlighted. + // + // Thus, we only treat the user as pressing ctrl-enter when the user presses + // ctrl without any fragile state built up in the omnibox: + // * the contents of the omnibox have not changed since the keypress, + // * there is no autocompleted text visible, and + // * the user is not typing a keyword query. + return (control_key_state_ == DOWN_WITHOUT_CHANGE && + inline_autocomplete_text_.empty() && !KeywordIsSelected())? std::wstring(L"com") : std::wstring(); } @@ -705,18 +718,20 @@ void AutocompleteEditModel::InternalSetUserText(const std::wstring& text) { inline_autocomplete_text_.clear(); } +bool AutocompleteEditModel::KeywordIsSelected() const { + return ((keyword_ui_state_ != NO_KEYWORD) && !is_keyword_hint_ && + !keyword_.empty()); +} + std::wstring AutocompleteEditModel::DisplayTextFromUserText( const std::wstring& text) const { - return ((keyword_ui_state_ == NO_KEYWORD) || is_keyword_hint_ || - keyword_.empty()) ? - text : KeywordProvider::SplitReplacementStringFromInput(text); + return KeywordIsSelected() ? + KeywordProvider::SplitReplacementStringFromInput(text) : text; } std::wstring AutocompleteEditModel::UserTextFromDisplayText( const std::wstring& text) const { - return ((keyword_ui_state_ == NO_KEYWORD) || is_keyword_hint_ || - keyword_.empty()) ? - text : (keyword_ + L" " + text); + return KeywordIsSelected() ? (keyword_ + L" " + text) : text; } void AutocompleteEditModel::GetInfoForCurrentText( diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h index 783b396..99374a6 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.h +++ b/chrome/browser/autocomplete/autocomplete_edit.h @@ -327,6 +327,9 @@ class AutocompleteEditModel : public NotificationObserver { // Called whenever user_text_ should change. void InternalSetUserText(const std::wstring& text); + // Returns true if a keyword is selected. + bool KeywordIsSelected() const; + // Conversion between user text and display text. User text is the text the // user has input. Display text is the text being shown in the edit. The // two are different if a keyword is selected. diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc index b060555..9d4ee13 100644 --- a/chrome/browser/autocomplete/history_url_provider.cc +++ b/chrome/browser/autocomplete/history_url_provider.cc @@ -403,8 +403,12 @@ bool HistoryURLProvider::FixupExactSuggestion(history::URLDatabase* db, return false; GURL destination_url(URLFixerUpper::FixupURL(WideToUTF8(input.text()), std::string())); - if (!db->GetRowForURL(destination_url, &info)) + if (!db->GetRowForURL(destination_url, NULL)) return false; + + // If we got here, then we hit the tricky corner case. Make sure that + // |info| corresponds to the right URL. + info = history::URLRow(match->destination_url); } else { // We have data for this match, use it. match->deletable = true; diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc index 393cd62..ae8f376 100644 --- a/chrome/browser/autocomplete/history_url_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc @@ -88,6 +88,11 @@ static TestURLInfo test_db[] = { {"http://www.\xEA\xB5\x90\xEC\x9C\xA1.kr/", "Korean", 2, 2}, {"http://spaces.com/path%20with%20spaces/foo.html", "Spaces", 2, 2}, {"http://ms/c++%20style%20guide", "Style guide", 2, 2}, + + // URLs for testing ctrl-enter behavior. + {"http://binky/", "Intranet binky", 2, 2}, + {"http://winky/", "Intranet winky", 2, 2}, + {"http://www.winky.com/", "Internet winky", 5, 0}, }; class HistoryURLProviderTest : public testing::Test, @@ -192,7 +197,8 @@ void HistoryURLProviderTest::RunTest(const std::wstring text, MessageLoop::current()->Run(); matches_ = autocomplete_->matches(); - ASSERT_EQ(num_results, matches_.size()) << "Input text: " << text; + ASSERT_EQ(num_results, matches_.size()) << "Input text: " << text + << "\nTLD: \"" << desired_tld << "\""; for (size_t i = 0; i < num_results; ++i) EXPECT_EQ(expected_urls[i], matches_[i].destination_url.spec()); } @@ -360,6 +366,35 @@ TEST_F(HistoryURLProviderTest, WhatYouTyped) { const std::string results_3[] = {"https://wytmatch%20foo%20bar/"}; RunTest(L"https://wytmatch foo bar", std::wstring(), false, results_3, arraysize(results_3)); + + // Test the corner case where a user has fully typed a previously visited + // intranet address and is now hitting ctrl-enter, which completes to a + // previously unvisted internet domain. + const std::string binky_results[] = {"http://binky/"}; + const std::string binky_com_results[] = { + "http://www.binky.com/", + "http://binky/", + }; + RunTest(L"binky", std::wstring(), false, binky_results, + arraysize(binky_results)); + RunTest(L"binky", L"com", false, binky_com_results, + arraysize(binky_com_results)); + + // Test the related case where a user has fully typed a previously visited + // intranet address and is now hitting ctrl-enter, which completes to a + // previously visted internet domain. + const std::string winky_results[] = { + "http://winky/", + "http://www.winky.com/", + }; + const std::string winky_com_results[] = { + "http://www.winky.com/", + "http://winky/", + }; + RunTest(L"winky", std::wstring(), false, winky_results, + arraysize(winky_results)); + RunTest(L"winky", L"com", false, winky_com_results, + arraysize(winky_com_results)); } TEST_F(HistoryURLProviderTest, Fixup) { |