diff options
author | mpcomplete@chromium.org <mpcomplete@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-18 00:15:38 +0000 |
---|---|---|
committer | mpcomplete@chromium.org <mpcomplete@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-06-18 00:15:38 +0000 |
commit | a5a43a6d55ff3aaf07f950826a89122e93e128fc (patch) | |
tree | ed73744df2f71fca48c0a0d8f43af0a2a5a88489 /chrome/browser/autocomplete | |
parent | 5e58fa046264b48a6513009ede03aa8d8d69cc6d (diff) | |
download | chromium_src-a5a43a6d55ff3aaf07f950826a89122e93e128fc.zip chromium_src-a5a43a6d55ff3aaf07f950826a89122e93e128fc.tar.gz chromium_src-a5a43a6d55ff3aaf07f950826a89122e93e128fc.tar.bz2 |
Fix a bug where clicking an extension omnibox keyword suggestion would
navigate you to an error page.
BUG=46480
Review URL: http://codereview.chromium.org/2819006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50178 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/autocomplete')
5 files changed, 58 insertions, 49 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc index 976e2e0..07f584c 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.cc +++ b/chrome/browser/autocomplete/autocomplete_edit.cc @@ -266,16 +266,6 @@ void AutocompleteEditModel::AcceptInput(WindowOpenDisposition disposition, GURL alternate_nav_url; GetInfoForCurrentText(&match, &alternate_nav_url); - if (match.template_url && match.template_url->IsExtensionKeyword()) { - // Strip the keyword + leading space off the input. - size_t prefix_length = match.template_url->keyword().size() + 1; - ExtensionOmniboxEventRouter::OnInputEntered( - profile_, match.template_url->GetExtensionId(), - WideToUTF8(match.fill_into_edit.substr(prefix_length))); - view_->RevertAll(); - return; - } - if (!match.destination_url.is_valid()) return; @@ -299,14 +289,18 @@ void AutocompleteEditModel::AcceptInput(WindowOpenDisposition disposition, is_keyword_hint_ ? std::wstring() : keyword_); } -void AutocompleteEditModel::SendOpenNotification(size_t selected_line, - const std::wstring& keyword) { +void AutocompleteEditModel::OpenURL(const GURL& url, + WindowOpenDisposition disposition, + PageTransition::Type transition, + const GURL& alternate_nav_url, + size_t index, + const std::wstring& keyword) { // We only care about cases where there is a selection (i.e. the popup is // open). if (popup_->IsOpen()) { scoped_ptr<AutocompleteLog> log(popup_->GetAutocompleteLog()); - if (selected_line != AutocompletePopupModel::kNoMatch) - log->selected_index = selected_line; + if (index != AutocompletePopupModel::kNoMatch) + log->selected_index = index; else if (!has_temporary_text_) log->inline_autocompleted_length = inline_autocomplete_text_.length(); NotificationService::current()->Notify( @@ -315,18 +309,42 @@ void AutocompleteEditModel::SendOpenNotification(size_t selected_line, } TemplateURLModel* template_url_model = profile_->GetTemplateURLModel(); - if (keyword.empty() || !template_url_model) - return; + if (template_url_model && !keyword.empty()) { + const TemplateURL* const template_url = + template_url_model->GetTemplateURLForKeyword(keyword); + + // Special case for extension keywords. Don't increment usage count for + // these. + if (template_url && template_url->IsExtensionKeyword()) { + AutocompleteMatch current_match; + GetInfoForCurrentText(¤t_match, NULL); + + const AutocompleteMatch& match = + index == AutocompletePopupModel::kNoMatch ? + current_match : result().match_at(index); + + // Strip the keyword + leading space off the input. + size_t prefix_length = match.template_url->keyword().size() + 1; + ExtensionOmniboxEventRouter::OnInputEntered( + profile_, match.template_url->GetExtensionId(), + WideToUTF8(match.fill_into_edit.substr(prefix_length))); + view_->RevertAll(); + return; + } + + if (template_url) { + UserMetrics::RecordAction(UserMetricsAction("AcceptedKeyword"), profile_); + template_url_model->IncrementUsageCount(template_url); + } - const TemplateURL* const template_url = - template_url_model->GetTemplateURLForKeyword(keyword); - if (template_url) { - UserMetrics::RecordAction(UserMetricsAction("AcceptedKeyword"), profile_); - template_url_model->IncrementUsageCount(template_url); + // NOTE: We purposefully don't increment the usage count of the default + // search engine, if applicable; see comments in template_url.h. } - // NOTE: We purposefully don't increment the usage count of the default search - // engine, if applicable; see comments in template_url.h. + if (disposition != NEW_BACKGROUND_TAB) + view_->RevertAll(); // Revert the box to its unedited state + controller_->OnAutocompleteAccept(url, disposition, transition, + alternate_nav_url); } void AutocompleteEditModel::AcceptKeyword() { diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h index 41e0d85..a194df8 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.h +++ b/chrome/browser/autocomplete/autocomplete_edit.h @@ -206,14 +206,13 @@ class AutocompleteEditModel : public NotificationObserver { void AcceptInput(WindowOpenDisposition disposition, bool for_drop); - // As necessary, sends out notification that the user is accepting a URL in - // the edit. If the accepted URL is from selecting a keyword, |keyword| is - // the selected keyword. - // If |selected_line| is kNoMatch, the currently selected line is used for the - // metrics log record; otherwise, the provided value is used as the selected - // line. This is used when the user opens a URL without actually selecting - // its entry, such as middle-clicking it. - void SendOpenNotification(size_t selected_line, const std::wstring& keyword); + // Asks the browser to load the item at |index|, with the given properties. + void OpenURL(const GURL& url, + WindowOpenDisposition disposition, + PageTransition::Type transition, + const GURL& alternate_nav_url, + size_t index, + const std::wstring& keyword); bool has_focus() const { return has_focus_; } diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc index 06f6ef7..54ab66e 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc @@ -394,12 +394,8 @@ void AutocompleteEditViewGtk::OpenURL(const GURL& url, if (!url.is_valid()) return; - model_->SendOpenNotification(selected_line, keyword); - - if (disposition != NEW_BACKGROUND_TAB) - RevertAll(); // Revert the box to its unedited state. - controller_->OnAutocompleteAccept(url, disposition, transition, - alternate_nav_url); + model_->OpenURL(url, disposition, transition, alternate_nav_url, + selected_line, keyword); } std::wstring AutocompleteEditViewGtk::GetText() const { diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm index b565e98..fc13cc6 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm +++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm @@ -285,12 +285,8 @@ void AutocompleteEditViewMac::OpenURL(const GURL& url, return; } - model_->SendOpenNotification(selected_line, keyword); - - if (disposition != NEW_BACKGROUND_TAB) - RevertAll(); // Revert the box to its unedited state. - controller_->OnAutocompleteAccept(url, disposition, transition, - alternate_nav_url); + model_->OpenURL(url, disposition, transition, alternate_nav_url, + selected_line, keyword); } std::wstring AutocompleteEditViewMac::GetText() const { diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc index 38bc75e..486d382 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc @@ -590,13 +590,13 @@ void AutocompleteEditViewWin::OpenURL(const GURL& url, if (!url.is_valid()) return; - model_->SendOpenNotification(selected_line, keyword); - + // When we navigate, we first revert to the unedited state, then if necessary + // synchronously change the permanent text to the new URL. If we don't freeze + // here, the user could potentially see a flicker of the current URL before + // the new one reappears, which would look glitchy. ScopedFreeze freeze(this, GetTextObjectModel()); - if (disposition != NEW_BACKGROUND_TAB) - RevertAll(); // Revert the box to its unedited state - controller_->OnAutocompleteAccept(url, disposition, transition, - alternate_nav_url); + model_->OpenURL(url, disposition, transition, alternate_nav_url, + selected_line, keyword); } std::wstring AutocompleteEditViewWin::GetText() const { |