diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-30 00:19:18 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-30 00:19:18 +0000 |
commit | 66ee443252fd759c5e20cb93be1e90a732da0ebe (patch) | |
tree | 5ee39c6ee0c62c8feba5103836a36d28203b7b54 /chrome/browser/autocomplete | |
parent | cbadedd7a2cdc20a745ff8b51922b2533169f7b0 (diff) | |
download | chromium_src-66ee443252fd759c5e20cb93be1e90a732da0ebe.zip chromium_src-66ee443252fd759c5e20cb93be1e90a732da0ebe.tar.gz chromium_src-66ee443252fd759c5e20cb93be1e90a732da0ebe.tar.bz2 |
Show the location bar icon (almost) all the time, and have its contents match what the user is doing.
There are a couple major moving parts here:
* Change AutocompletePopupModel::URLsForCurrentText() to InfoForCurrentText() and have it return an AutocompleteMatch, which callers can use to parse out whatever they want. I needed to get at the match type for the current text and found the proliferation of arguments here ridiculous. This had major ripple effects throughout the codebase, including changing the name and location of SearchVersusNavigateClassifier as it no longer had an "is_search" parameter directly, so the name became misleading and too narrow. I also ended up adding a null constructor for AutocompleteMatch because it was too cumbersome otherwise.
* Change the name of the "SecurityImageView" (or similar) to reflect its broader purpose, and plumb it to the edit to get an icon instead of to the toolbar model.
* Add an AutocompleteMatch::Type to icon mapping function, and use it not only in the new code but also to simplify showing the popup contents.
BUG=27570,39725
TEST=An icon should appear next to the address at all times. It should be a globe on non-secure pages, a magnifying glass on the NTP, and a match for whatever the user is typing as he types.
Review URL: http://codereview.chromium.org/1457002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@43025 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/autocomplete')
18 files changed, 292 insertions, 231 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 2167ea2..db1655b 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -27,6 +27,7 @@ #include "googleurl/src/url_canon_ip.h" #include "googleurl/src/url_util.h" #include "grit/generated_resources.h" +#include "grit/theme_resources.h" #include "net/base/net_util.h" #include "net/base/registry_controlled_domain.h" #include "net/url_request/url_request.h" @@ -367,6 +368,18 @@ void AutocompleteInput::Clear() { // AutocompleteMatch ---------------------------------------------------------- +AutocompleteMatch::AutocompleteMatch() + : provider(NULL), + relevance(0), + deletable(false), + inline_autocomplete_offset(std::wstring::npos), + transition(PageTransition::GENERATED), + is_history_what_you_typed_match(false), + type(SEARCH_WHAT_YOU_TYPED), + template_url(NULL), + starred(false) { +} + AutocompleteMatch::AutocompleteMatch(AutocompleteProvider* provider, int relevance, bool deletable, @@ -384,23 +397,40 @@ AutocompleteMatch::AutocompleteMatch(AutocompleteProvider* provider, // static std::string AutocompleteMatch::TypeToString(Type type) { - switch (type) { - case URL_WHAT_YOU_TYPED: return "url-what-you-typed"; - case HISTORY_URL: return "history-url"; - case HISTORY_TITLE: return "history-title"; - case HISTORY_BODY: return "history-body"; - case HISTORY_KEYWORD: return "history-keyword"; - case NAVSUGGEST: return "navsuggest"; - case SEARCH_WHAT_YOU_TYPED: return "search-what-you-typed"; - case SEARCH_HISTORY: return "search-history"; - case SEARCH_SUGGEST: return "search-suggest"; - case SEARCH_OTHER_ENGINE: return "search-other-engine"; - case OPEN_HISTORY_PAGE: return "open-history-page"; + const char* strings[NUM_TYPES] = { + "url-what-you-typed", + "history-url", + "history-title", + "history-body", + "history-keyword", + "navsuggest", + "search-what-you-typed", + "search-history", + "search-suggest", + "search-other-engine", + "open-history-page", + }; + DCHECK(arraysize(strings) == NUM_TYPES); + return strings[type]; +} - default: - NOTREACHED(); - return std::string(); - } +// static +int AutocompleteMatch::TypeToIcon(Type type) { + int icons[NUM_TYPES] = { + IDR_O2_GLOBE, + IDR_O2_GLOBE, + IDR_O2_HISTORY, + IDR_O2_HISTORY, + IDR_O2_HISTORY, + IDR_O2_GLOBE, + IDR_O2_SEARCH, + IDR_O2_SEARCH, + IDR_O2_SEARCH, + IDR_O2_SEARCH, + IDR_O2_MORE, + }; + DCHECK(arraysize(icons) == NUM_TYPES); + return icons[type]; } // static diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h index d64fd6c..124783f1 100644 --- a/chrome/browser/autocomplete/autocomplete.h +++ b/chrome/browser/autocomplete/autocomplete.h @@ -314,22 +314,24 @@ struct AutocompleteMatch { // The type of this match. enum Type { - URL_WHAT_YOU_TYPED, // The input as a URL. - HISTORY_URL, // A past page whose URL contains the input. - HISTORY_TITLE, // A past page whose title contains the input. - HISTORY_BODY, // A past page whose body contains the input. - HISTORY_KEYWORD, // A past page whose keyword contains the input. - NAVSUGGEST, // A suggested URL. - SEARCH_WHAT_YOU_TYPED, // The input as a search query (with the default - // engine). - SEARCH_HISTORY, // A past search (with the default engine) - // containing the input. - SEARCH_SUGGEST, // A suggested search (with the default engine). - SEARCH_OTHER_ENGINE, // A search with a non-default engine. - OPEN_HISTORY_PAGE, // A synthetic result that opens the history page to - // search for the input. + URL_WHAT_YOU_TYPED = 0, // The input as a URL. + HISTORY_URL, // A past page whose URL contains the input. + HISTORY_TITLE, // A past page whose title contains the input. + HISTORY_BODY, // A past page whose body contains the input. + HISTORY_KEYWORD, // A past page whose keyword contains the input. + NAVSUGGEST, // A suggested URL. + SEARCH_WHAT_YOU_TYPED, // The input as a search query (with the default + // engine). + SEARCH_HISTORY, // A past search (with the default engine) + // containing the input. + SEARCH_SUGGEST, // A suggested search (with the default engine). + SEARCH_OTHER_ENGINE, // A search with a non-default engine. + OPEN_HISTORY_PAGE, // A synthetic result that opens the history page + // to search for the input. + NUM_TYPES, }; + AutocompleteMatch(); AutocompleteMatch(AutocompleteProvider* provider, int relevance, bool deletable, @@ -338,6 +340,10 @@ struct AutocompleteMatch { // Converts |type| to a string representation. Used in logging. static std::string TypeToString(Type type); + // Converts |type| to a resource identifier for the appropriate icon for this + // type. + static int TypeToIcon(Type type); + // Comparison function for determining when one match is better than another. static bool MoreRelevant(const AutocompleteMatch& elem1, const AutocompleteMatch& elem2); @@ -776,7 +782,7 @@ class AutocompleteController : public ACProviderListener { const AutocompleteInput& input() const { return input_; } const AutocompleteResult& result() const { return result_; } // This next is temporary and should go away when - // AutocompletePopup::URLsForCurrentSelection() moves to the controller. + // AutocompletePopup::InfoForCurrentSelection() moves to the controller. const AutocompleteResult& latest_result() const { return latest_result_; } bool done() const { return done_ && !update_delay_timer_.IsRunning(); } diff --git a/chrome/browser/autocomplete/autocomplete_classifier.cc b/chrome/browser/autocomplete/autocomplete_classifier.cc new file mode 100644 index 0000000..3e96ff5 --- /dev/null +++ b/chrome/browser/autocomplete/autocomplete_classifier.cc @@ -0,0 +1,34 @@ +// Copyright (c) 2010 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. + +#include "chrome/browser/autocomplete/autocomplete_classifier.h" + +#include "chrome/browser/autocomplete/autocomplete.h" +#include "googleurl/src/gurl.h" + +AutocompleteClassifier::AutocompleteClassifier(Profile* profile) + : controller_(new AutocompleteController(profile)) { +} + +AutocompleteClassifier::~AutocompleteClassifier() { +} + +void AutocompleteClassifier::Classify(const std::wstring& text, + const std::wstring& desired_tld, + AutocompleteMatch* match, + GURL* alternate_nav_url) { + controller_->Start(text, desired_tld, true, false, true); + DCHECK(controller_->done()); + const AutocompleteResult& result = controller_->result(); + if (result.empty()) { + if (alternate_nav_url) + *alternate_nav_url = GURL(); + return; + } + + DCHECK(result.default_match() != result.end()); + *match = *result.default_match(); + if (alternate_nav_url) + *alternate_nav_url = result.alternate_nav_url(); +} diff --git a/chrome/browser/autocomplete/autocomplete_classifier.h b/chrome/browser/autocomplete/autocomplete_classifier.h new file mode 100644 index 0000000..3588c27 --- /dev/null +++ b/chrome/browser/autocomplete/autocomplete_classifier.h @@ -0,0 +1,43 @@ +// Copyright (c) 2010 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. + +#ifndef CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_CLASSIFIER_H_ +#define CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_CLASSIFIER_H_ + +#include <string> + +#include "base/basictypes.h" +#include "base/scoped_ptr.h" + +class AutocompleteController; +struct AutocompleteMatch; +class GURL; +class Profile; + +class AutocompleteClassifier { + public: + explicit AutocompleteClassifier(Profile* profile); + virtual ~AutocompleteClassifier(); + + // Given some string |text| that the user wants to use for navigation, + // determines how it should be interpreted. |desired_tld| is the user's + // desired TLD, if any; see AutocompleteInput::desired_tld(). |match| should + // be a non-NULL outparam that will be set to the default match for this + // input, if any (for invalid input, there will be no default match, and + // |match| will be left unchanged). |alternate_nav_url| is a possibly-NULL + // outparam that, if non-NULL, will be set to the navigational URL (if any) in + // case of an accidental search; see comments on + // AutocompleteResult::alternate_nav_url_ in autocomplete.h. + void Classify(const std::wstring& text, + const std::wstring& desired_tld, + AutocompleteMatch* match, + GURL* alternate_nav_url); + + private: + scoped_ptr<AutocompleteController> controller_; + + DISALLOW_IMPLICIT_CONSTRUCTORS(AutocompleteClassifier); +}; + +#endif // CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_CLASSIFIER_H_ diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc index f4c2583..22c6802 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.cc +++ b/chrome/browser/autocomplete/autocomplete_edit.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -9,6 +9,7 @@ #include "base/basictypes.h" #include "base/utf_string_conversions.h" #include "chrome/app/chrome_dll_resource.h" +#include "chrome/browser/autocomplete/autocomplete_classifier.h" #include "chrome/browser/autocomplete/autocomplete_edit_view.h" #include "chrome/browser/autocomplete/autocomplete_popup_model.h" #include "chrome/browser/autocomplete/keyword_provider.h" @@ -19,7 +20,6 @@ #include "chrome/browser/profile.h" #include "chrome/browser/search_engines/template_url.h" #include "chrome/browser/search_engines/template_url_model.h" -#include "chrome/browser/search_versus_navigate_classifier.h" #include "chrome/common/notification_service.h" #include "googleurl/src/gurl.h" #include "googleurl/src/url_util.h" @@ -122,7 +122,9 @@ void AutocompleteEditModel::SetUserText(const std::wstring& text) { void AutocompleteEditModel::GetDataForURLExport(GURL* url, std::wstring* title, SkBitmap* favicon) { - *url = GetURLForCurrentText(NULL, NULL, NULL); + AutocompleteMatch match; + GetInfoForCurrentText(&match, NULL); + *url = match.destination_url; if (UTF8ToWide(url->possibly_invalid_spec()) == permanent_text_) { *title = controller_->GetTitle(); *favicon = controller_->GetFavIcon(); @@ -134,7 +136,7 @@ std::wstring AutocompleteEditModel::GetDesiredTLD() const { std::wstring(L"com") : std::wstring(); } -bool AutocompleteEditModel::CurrentTextIsURL() { +bool AutocompleteEditModel::CurrentTextIsURL() const { // If !user_input_in_progress_, the permanent text is showing, which should // always be a URL, so no further checking is needed. By avoiding checking in // this case, we avoid calling into the autocomplete providers, and thus @@ -142,9 +144,15 @@ bool AutocompleteEditModel::CurrentTextIsURL() { if (!user_input_in_progress_) return true; - PageTransition::Type transition = PageTransition::LINK; - GetURLForCurrentText(&transition, NULL, NULL); - return transition == PageTransition::TYPED; + AutocompleteMatch match; + GetInfoForCurrentText(&match, NULL); + return match.transition == PageTransition::TYPED; +} + +AutocompleteMatch::Type AutocompleteEditModel::CurrentTextType() const { + AutocompleteMatch match; + GetInfoForCurrentText(&match, NULL); + return match.type; } bool AutocompleteEditModel::GetURLForText(const std::wstring& text, @@ -191,14 +199,11 @@ bool AutocompleteEditModel::CanPasteAndGo(const std::wstring& text) const { if (!view_->GetCommandUpdater()->IsCommandEnabled(IDC_OPEN_CURRENT_URL)) return false; - paste_and_go_url_ = GURL(); - paste_and_go_transition_ = PageTransition::TYPED; - paste_and_go_alternate_nav_url_ = GURL(); - - profile_->GetSearchVersusNavigateClassifier()->Classify(text, std::wstring(), - NULL, &paste_and_go_url_, &paste_and_go_transition_, NULL, - &paste_and_go_alternate_nav_url_); - + AutocompleteMatch match; + profile_->GetAutocompleteClassifier()->Classify(text, std::wstring(), + &match, &paste_and_go_alternate_nav_url_); + paste_and_go_url_ = match.destination_url; + paste_and_go_transition_ = match.transition; return paste_and_go_url_.is_valid(); } @@ -215,33 +220,30 @@ void AutocompleteEditModel::PasteAndGo() { void AutocompleteEditModel::AcceptInput(WindowOpenDisposition disposition, bool for_drop) { // Get the URL and transition type for the selected entry. - PageTransition::Type transition; - bool is_history_what_you_typed_match; + AutocompleteMatch match; GURL alternate_nav_url; - const GURL url(GetURLForCurrentText(&transition, - &is_history_what_you_typed_match, - &alternate_nav_url)); - if (!url.is_valid()) + GetInfoForCurrentText(&match, &alternate_nav_url); + if (!match.destination_url.is_valid()) return; - if (UTF8ToWide(url.spec()) == permanent_text_) { + if (UTF8ToWide(match.destination_url.spec()) == permanent_text_) { // When the user hit enter on the existing permanent URL, treat it like a // reload for scoring purposes. We could detect this by just checking // user_input_in_progress_, but it seems better to treat "edits" that end // up leaving the URL unchanged (e.g. deleting the last character and then // retyping it) as reloads too. - transition = PageTransition::RELOAD; + match.transition = PageTransition::RELOAD; } else if (for_drop || ((paste_state_ != NONE) && - is_history_what_you_typed_match)) { + match.is_history_what_you_typed_match)) { // When the user pasted in a URL and hit enter, score it like a link click // rather than a normal typed URL, so it doesn't get inline autocompleted // as aggressively later. - transition = PageTransition::LINK; + match.transition = PageTransition::LINK; } - view_->OpenURL(url, disposition, transition, alternate_nav_url, - AutocompletePopupModel::kNoMatch, - is_keyword_hint_ ? std::wstring() : keyword_); + view_->OpenURL(match.destination_url, disposition, match.transition, + alternate_nav_url, AutocompletePopupModel::kNoMatch, + is_keyword_hint_ ? std::wstring() : keyword_); } void AutocompleteEditModel::SendOpenNotification(size_t selected_line, @@ -325,17 +327,20 @@ void AutocompleteEditModel::OnKillFocus() { } bool AutocompleteEditModel::OnEscapeKeyPressed() { - if (has_temporary_text_ && - (popup_->URLsForCurrentSelection(NULL, NULL, NULL) != original_url_)) { - // The user typed something, then selected a different item. Restore the - // text they typed and change back to the default item. - // NOTE: This purposefully does not reset paste_state_. - just_deleted_text_ = false; - has_temporary_text_ = false; - keyword_ui_state_ = original_keyword_ui_state_; - popup_->ResetToDefaultMatch(); - view_->OnRevertTemporaryText(); - return true; + if (has_temporary_text_) { + AutocompleteMatch match; + popup_->InfoForCurrentSelection(&match, NULL); + if (match.destination_url != original_url_) { + // The user typed something, then selected a different item. Restore the + // text they typed and change back to the default item. + // NOTE: This purposefully does not reset paste_state_. + just_deleted_text_ = false; + has_temporary_text_ = false; + keyword_ui_state_ = original_keyword_ui_state_; + popup_->ResetToDefaultMatch(); + view_->OnRevertTemporaryText(); + return true; + } } // If the user wasn't editing, but merely had focus in the edit, allow <esc> @@ -405,7 +410,7 @@ void AutocompleteEditModel::OnUpOrDownKeyPressed(int count) { void AutocompleteEditModel::OnPopupDataChanged( const std::wstring& text, - bool is_temporary_text, + GURL* destination_for_temporary_text_change, const std::wstring& keyword, bool is_keyword_hint, AutocompleteMatch::Type type) { @@ -428,12 +433,12 @@ void AutocompleteEditModel::OnPopupDataChanged( } // Handle changes to temporary text. - if (is_temporary_text) { + if (destination_for_temporary_text_change != NULL) { const bool save_original_selection = !has_temporary_text_; if (save_original_selection) { // Save the original selection and URL so it can be reverted later. has_temporary_text_ = true; - original_url_ = popup_->URLsForCurrentSelection(NULL, NULL, NULL); + original_url_ = *destination_for_temporary_text_change; original_keyword_ui_state_ = keyword_ui_state_; } if (control_key_state_ == DOWN_WITHOUT_CHANGE) { @@ -562,7 +567,7 @@ void AutocompleteEditModel::Observe(NotificationType type, match_type = match->type; } - OnPopupDataChanged(inline_autocomplete_text, false, keyword, is_keyword_hint, + OnPopupDataChanged(inline_autocomplete_text, NULL, keyword, is_keyword_hint, match_type); } @@ -586,20 +591,14 @@ std::wstring AutocompleteEditModel::UserTextFromDisplayText( text : (keyword_ + L" " + text); } -GURL AutocompleteEditModel::GetURLForCurrentText( - PageTransition::Type* transition, - bool* is_history_what_you_typed_match, +void AutocompleteEditModel::GetInfoForCurrentText( + AutocompleteMatch* match, GURL* alternate_nav_url) const { if (popup_->IsOpen() || query_in_progress()) { - return popup_->URLsForCurrentSelection(transition, - is_history_what_you_typed_match, - alternate_nav_url); + popup_->InfoForCurrentSelection(match, alternate_nav_url); + } else { + profile_->GetAutocompleteClassifier()->Classify( + UserTextFromDisplayText(view_->GetText()), GetDesiredTLD(), match, + alternate_nav_url); } - - GURL destination_url; - profile_->GetSearchVersusNavigateClassifier()->Classify( - UserTextFromDisplayText(view_->GetText()), GetDesiredTLD(), NULL, - &destination_url, transition, is_history_what_you_typed_match, - alternate_nav_url); - return destination_url; } diff --git a/chrome/browser/autocomplete/autocomplete_edit.h b/chrome/browser/autocomplete/autocomplete_edit.h index 1a0386c..96a86c7 100644 --- a/chrome/browser/autocomplete/autocomplete_edit.h +++ b/chrome/browser/autocomplete/autocomplete_edit.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -137,7 +137,10 @@ class AutocompleteEditModel : public NotificationObserver { // Returns true if the current edit contents will be treated as a // URL/navigation, as opposed to a search. - bool CurrentTextIsURL(); + bool CurrentTextIsURL() const; + + // Returns the match type for the current edit contents. + AutocompleteMatch::Type CurrentTextType() const; // Returns true if |text| (which is display text in the current context) // parses as a URL, and in that case sets |url| to the calculated URL. @@ -208,7 +211,7 @@ class AutocompleteEditModel : public NotificationObserver { // Accessors for keyword-related state (see comments on keyword_ and // is_keyword_hint_). std::wstring keyword() const { - return (is_keyword_hint_ ? has_focus_ : (keyword_ui_state_ != NO_KEYWORD)) ? + return (is_keyword_hint_ || (keyword_ui_state_ != NO_KEYWORD)) ? keyword_ : std::wstring(); } bool is_keyword_hint() const { return is_keyword_hint_; } @@ -222,7 +225,7 @@ class AutocompleteEditModel : public NotificationObserver { // True if we should show the "Type to search" hint (see comments on // show_search_hint_). - bool show_search_hint() const { return has_focus_ && show_search_hint_; } + bool show_search_hint() const { return show_search_hint_; } // Returns true if a query to an autocomplete provider is currently // in progress. This logic should in the future live in @@ -260,9 +263,12 @@ class AutocompleteEditModel : public NotificationObserver { // Called when any relevant data changes. This rolls together several // separate pieces of data into one call so we can update all the UI // efficiently: - // |text| is either the new temporary text (if |is_temporary_text| is true) - // from the user manually selecting a different match, or the inline - // autocomplete text (if |is_temporary_text| is false). + // |text| is either the new temporary text from the user manually selecting + // a different match, or the inline autocomplete text. We distinguish by + // checking if |destination_for_temporary_text_change| is NULL. + // |destination_for_temporary_text_change| is NULL (if temporary text should + // not change) or the pre-change desitnation URL (if temporary text should + // change) so we can save it off to restore later. // |keyword| is the keyword to show a hint for if |is_keyword_hint| is true, // or the currently selected keyword if |is_keyword_hint| is false (see // comments on keyword_ and is_keyword_hint_). @@ -271,7 +277,7 @@ class AutocompleteEditModel : public NotificationObserver { // show_search_hint_). void OnPopupDataChanged( const std::wstring& text, - bool is_temporary_text, + GURL* destination_for_temporary_text_change, const std::wstring& keyword, bool is_keyword_hint, AutocompleteMatch::Type type); @@ -326,16 +332,10 @@ class AutocompleteEditModel : public NotificationObserver { std::wstring DisplayTextFromUserText(const std::wstring& text) const; std::wstring UserTextFromDisplayText(const std::wstring& text) const; - // Returns the URL. If the user has not edited the text, this returns the - // permanent text. If the user has edited the text, this returns the default - // match based on the current text, which may be a search URL, or keyword - // generated URL. - // - // See AutocompleteEdit for a description of the args (they may be null if - // not needed). - GURL GetURLForCurrentText(PageTransition::Type* transition, - bool* is_history_what_you_typed_match, - GURL* alternate_nav_url) const; + // Returns the default match for the current text, as well as the alternate + // nav URL, if |alternate_nav_url| is non-NULL and there is such a URL. + void GetInfoForCurrentText(AutocompleteMatch* match, + GURL* alternate_nav_url) const; AutocompleteEditView* view_; diff --git a/chrome/browser/autocomplete/autocomplete_edit_view.h b/chrome/browser/autocomplete/autocomplete_edit_view.h index 63c2524..e29b19b 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view.h +++ b/chrome/browser/autocomplete/autocomplete_edit_view.h @@ -59,6 +59,9 @@ class AutocompleteEditView { // browser, or just whatever the user has currently typed. virtual std::wstring GetText() const = 0; + // Returns the resource ID of the icon to show for the current text. + virtual int GetIcon() const = 0; + // The user text is the text the user has manually keyed in. When present, // this is shown in preference to the permanent text; hitting escape will // revert to the permanent text. diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc index 1c7d4bd..0afafd2 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc @@ -387,6 +387,12 @@ std::wstring AutocompleteEditViewGtk::GetText() const { return out; } +int AutocompleteEditViewGtk::GetIcon() const { + return (model_->user_input_in_progress() || model_->show_search_hint()) ? + AutocompleteMatch::TypeToIcon(model_->CurrentTextType()) : + toolbar_model_->GetIcon(); +} + void AutocompleteEditViewGtk::SetUserText(const std::wstring& text, const std::wstring& display_text, bool update_popup) { diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h index 3632e0b..7065a8e 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h +++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h @@ -79,6 +79,8 @@ class AutocompleteEditViewGtk : public AutocompleteEditView, virtual std::wstring GetText() const; + virtual int GetIcon() const; + virtual void SetUserText(const std::wstring& text) { SetUserText(text, text, true); } diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h index 11bb62f..8539cae 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h +++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h @@ -48,6 +48,9 @@ class AutocompleteEditViewMac : public AutocompleteEditView, const std::wstring& keyword); virtual std::wstring GetText() const; + + virtual int GetIcon() const; + virtual void SetUserText(const std::wstring& text) { SetUserText(text, text, true); } diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm index 55b6f3f..1c49e43 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm +++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm @@ -266,6 +266,12 @@ std::wstring AutocompleteEditViewMac::GetText() const { return base::SysNSStringToWide([field_ stringValue]); } +int AutocompleteEditViewMac::GetIcon() const { + return (model_->user_input_in_progress() || model_->show_search_hint()) ? + AutocompleteMatch::TypeToIcon(model_->CurrentTextType()) : + toolbar_model_->GetIcon(); +} + void AutocompleteEditViewMac::SetUserText(const std::wstring& text, const std::wstring& display_text, bool update_popup) { diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc index 3bed00a..87c41f9 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_win.cc +++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.cc @@ -599,6 +599,12 @@ std::wstring AutocompleteEditViewWin::GetText() const { return str; } +int AutocompleteEditViewWin::GetIcon() const { + return (model_->user_input_in_progress() || model_->show_search_hint()) ? + AutocompleteMatch::TypeToIcon(model_->CurrentTextType()) : + toolbar_model_->GetIcon(); +} + void AutocompleteEditViewWin::SetUserText(const std::wstring& text, const std::wstring& display_text, bool update_popup) { diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_win.h b/chrome/browser/autocomplete/autocomplete_edit_view_win.h index 42e1a35..718ba2a 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_win.h +++ b/chrome/browser/autocomplete/autocomplete_edit_view_win.h @@ -91,6 +91,8 @@ class AutocompleteEditViewWin virtual std::wstring GetText() const; + virtual int GetIcon() const; + virtual void SetUserText(const std::wstring& text) { SetUserText(text, text, true); } diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc index eeca0d2..f769937 100644 --- a/chrome/browser/autocomplete/autocomplete_popup_model.cc +++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc @@ -104,12 +104,24 @@ void AutocompletePopupModel::SetSelectedLine(size_t line, if (line == selected_line_) return; // Nothing else to do. + // We need to update |selected_line_| before calling OnPopupDataChanged(), so + // that when the edit notifies its controller that something has changed, the + // controller can get the correct updated data. + // + // NOTE: We should never reach here with no selected line; the same code that + // opened the popup and made it possible to get here should have also set a + // selected line. + CHECK(selected_line_ != kNoMatch); + GURL current_destination(result.match_at(selected_line_).destination_url); + view_->InvalidateLine(selected_line_); + selected_line_ = line; + view_->InvalidateLine(selected_line_); + // Update the edit with the new data for this match. // TODO(pkasting): If |selected_line_| moves to the controller, this can be // eliminated and just become a call to the observer on the edit. std::wstring keyword; const bool is_keyword_hint = GetKeywordForMatch(match, &keyword); - if (reset_to_default) { std::wstring inline_autocomplete_text; if ((match.inline_autocomplete_offset != std::wstring::npos) && @@ -117,27 +129,15 @@ void AutocompletePopupModel::SetSelectedLine(size_t line, inline_autocomplete_text = match.fill_into_edit.substr(match.inline_autocomplete_offset); } - edit_model_->OnPopupDataChanged(inline_autocomplete_text, false, + edit_model_->OnPopupDataChanged(inline_autocomplete_text, NULL, keyword, is_keyword_hint, match.type); } else { - edit_model_->OnPopupDataChanged(match.fill_into_edit, true, + edit_model_->OnPopupDataChanged(match.fill_into_edit, ¤t_destination, keyword, is_keyword_hint, match.type); } // Repaint old and new selected lines immediately, so that the edit doesn't - // appear to update [much] faster than the popup. We must not update - // |selected_line_| before calling OnPopupDataChanged() (since the edit may - // call us back to get data about the old selection), and we must not call - // UpdateWindow() before updating |selected_line_| (since the paint routine - // relies on knowing the correct selected line). - // - // NOTE: We should never reach here with no selected line; the same code that - // opened the popup and made it possible to get here should have also set a - // selected line. - CHECK(selected_line_ != kNoMatch); - view_->InvalidateLine(selected_line_); - selected_line_ = line; - view_->InvalidateLine(selected_line_); + // appear to update [much] faster than the popup. view_->PaintUpdatesNow(); } @@ -148,22 +148,21 @@ void AutocompletePopupModel::ResetToDefaultMatch() { view_->OnDragCanceled(); } -GURL AutocompletePopupModel::URLsForCurrentSelection( - PageTransition::Type* transition, - bool* is_history_what_you_typed_match, +void AutocompletePopupModel::InfoForCurrentSelection( + AutocompleteMatch* match, GURL* alternate_nav_url) const { + DCHECK(match != NULL); const AutocompleteResult* result; - AutocompleteResult::const_iterator match; if (!controller_->done()) { result = &controller_->latest_result(); // It's technically possible for |result| to be empty if no provider returns // a synchronous result but the query has not completed synchronously; // pratically, however, that should never actually happen. if (result->empty()) - return GURL(); + return; // The user cannot have manually selected a match, or the query would have // stopped. So the default match must be the desired selection. - match = result->default_match(); + *match = *result->default_match(); } else { CHECK(IsOpen()); // The query isn't running, so the standard result set can't possibly be out @@ -178,15 +177,10 @@ GURL AutocompletePopupModel::URLsForCurrentSelection( // called instead. CHECK(!result->empty()); CHECK(selected_line_ < result->size()); - match = result->begin() + selected_line_; + *match = result->match_at(selected_line_); } - if (transition) - *transition = match->transition; - if (is_history_what_you_typed_match) - *is_history_what_you_typed_match = match->is_history_what_you_typed_match; if (alternate_nav_url && manually_selected_match_.empty()) *alternate_nav_url = result->alternate_nav_url(); - return match->destination_url; } bool AutocompletePopupModel::GetKeywordForMatch(const AutocompleteMatch& match, @@ -240,7 +234,7 @@ void AutocompletePopupModel::Move(int count) { } void AutocompletePopupModel::TryDeletingCurrentItem() { - // We could use URLsForCurrentSelection() here, but it seems better to try + // We could use InfoForCurrentSelection() here, but it seems better to try // and shift-delete the actual selection, rather than any "in progress, not // yet visible" one. if (selected_line_ == kNoMatch) diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.h b/chrome/browser/autocomplete/autocomplete_popup_model.h index 166a238..a986419 100644 --- a/chrome/browser/autocomplete/autocomplete_popup_model.h +++ b/chrome/browser/autocomplete/autocomplete_popup_model.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -73,15 +73,9 @@ class AutocompletePopupModel : public NotificationObserver { // will change the selected line back to the default match and redraw. void ResetToDefaultMatch(); - // Returns the URL for the selected match. If an update is in progress, - // "selected" means "default in the latest matches". If there are no - // matches, returns the empty string. - // - // If |transition_type| is non-NULL, it will be set to the appropriate - // transition type for the selected entry (TYPED or GENERATED). - // - // If |is_history_what_you_typed_match| is non-NULL, it will be set based on - // the selected entry's is_history_what_you_typed value. + // Copies the selected match into |match|. If an update is in progress, + // "selected" means "default in the latest matches". If there are no matches, + // does not update |match|. // // If |alternate_nav_url| is non-NULL, it will be set to the alternate // navigation URL for |url| if one exists, or left unchanged otherwise. See @@ -89,10 +83,8 @@ class AutocompletePopupModel : public NotificationObserver { // // TODO(pkasting): When manually_selected_match_ moves to the controller, this // can move too. - GURL URLsForCurrentSelection( - PageTransition::Type* transition, - bool* is_history_what_you_typed_match, - GURL* alternate_nav_url) const; + void InfoForCurrentSelection(AutocompleteMatch* match, + GURL* alternate_nav_url) const; // Gets the selected keyword or keyword hint for the given match. Returns // true if |keyword| represents a keyword hint, or false if |keyword| diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_popup_view_gtk.cc index 84538e0f..024d7d0 100644 --- a/chrome/browser/autocomplete/autocomplete_popup_view_gtk.cc +++ b/chrome/browser/autocomplete/autocomplete_popup_view_gtk.cc @@ -165,47 +165,21 @@ void SetupLayoutForMatch(PangoLayout* layout, } GdkPixbuf* IconForMatch(const AutocompleteMatch& match, bool selected) { + int icon = match.starred ? + IDR_O2_STAR : AutocompleteMatch::TypeToIcon(match.type); + if (selected) { + switch (icon) { + case IDR_O2_GLOBE: icon = IDR_O2_GLOBE_SELECTED; break; + case IDR_O2_HISTORY: icon = IDR_O2_HISTORY_SELECTED; break; + case IDR_O2_SEARCH: icon = IDR_O2_SEARCH_SELECTED; break; + case IDR_O2_MORE: icon = IDR_O2_MORE_SELECTED; break; + case IDR_O2_STAR: icon = IDR_O2_STAR_SELECTED; break; + default: NOTREACHED(); break; + } + } // TODO(deanm): These would be better as pixmaps someday. // TODO(estade): Do we want to flip these for RTL? (Windows doesn't). - ResourceBundle& rb = ResourceBundle::GetSharedInstance(); - static GdkPixbuf* o2_globe = rb.GetPixbufNamed(IDR_O2_GLOBE); - static GdkPixbuf* o2_globe_s = rb.GetPixbufNamed(IDR_O2_GLOBE_SELECTED_DARK); - static GdkPixbuf* o2_history = rb.GetPixbufNamed(IDR_O2_HISTORY); - static GdkPixbuf* o2_history_s = - rb.GetPixbufNamed(IDR_O2_HISTORY_SELECTED_DARK); - static GdkPixbuf* o2_more = rb.GetPixbufNamed(IDR_O2_MORE); - static GdkPixbuf* o2_more_s = rb.GetPixbufNamed(IDR_O2_MORE_SELECTED_DARK); - static GdkPixbuf* o2_search = rb.GetPixbufNamed(IDR_O2_SEARCH); - static GdkPixbuf* o2_search_s = - rb.GetPixbufNamed(IDR_O2_SEARCH_SELECTED_DARK); - static GdkPixbuf* o2_star = rb.GetPixbufNamed(IDR_O2_STAR); - static GdkPixbuf* o2_star_s = rb.GetPixbufNamed(IDR_O2_STAR_SELECTED_DARK); - - if (match.starred) - return selected ? o2_star_s : o2_star; - - switch (match.type) { - case AutocompleteMatch::URL_WHAT_YOU_TYPED: - case AutocompleteMatch::NAVSUGGEST: - return selected ? o2_globe_s : o2_globe; - case AutocompleteMatch::HISTORY_URL: - case AutocompleteMatch::HISTORY_TITLE: - case AutocompleteMatch::HISTORY_BODY: - case AutocompleteMatch::HISTORY_KEYWORD: - return selected ? o2_history_s : o2_history; - case AutocompleteMatch::SEARCH_WHAT_YOU_TYPED: - case AutocompleteMatch::SEARCH_HISTORY: - case AutocompleteMatch::SEARCH_SUGGEST: - case AutocompleteMatch::SEARCH_OTHER_ENGINE: - return selected ? o2_search_s : o2_search; - case AutocompleteMatch::OPEN_HISTORY_PAGE: - return selected ? o2_more_s : o2_more; - default: - NOTREACHED(); - break; - } - - return NULL; + return ResourceBundle::GetSharedInstance().GetPixbufNamed(icon); } } // namespace diff --git a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm index a25c381..6407475 100644 --- a/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm +++ b/chrome/browser/autocomplete/autocomplete_popup_view_mac.mm @@ -86,46 +86,6 @@ NSImage* RetainedResourceImage(int resource_id) { return [image retain]; } -// Return the appropriate icon for the given match. Derived from the -// gtk code. -NSImage* MatchIcon(const AutocompleteMatch& match) { - if (match.starred) { - static NSImage* starImage = RetainedResourceImage(IDR_O2_STAR); - return starImage; - } - - switch (match.type) { - case AutocompleteMatch::URL_WHAT_YOU_TYPED: - case AutocompleteMatch::NAVSUGGEST: { - static NSImage* globeImage = RetainedResourceImage(IDR_O2_GLOBE); - return globeImage; - } - case AutocompleteMatch::HISTORY_URL: - case AutocompleteMatch::HISTORY_TITLE: - case AutocompleteMatch::HISTORY_BODY: - case AutocompleteMatch::HISTORY_KEYWORD: { - static NSImage* historyImage = RetainedResourceImage(IDR_O2_HISTORY); - return historyImage; - } - case AutocompleteMatch::SEARCH_WHAT_YOU_TYPED: - case AutocompleteMatch::SEARCH_HISTORY: - case AutocompleteMatch::SEARCH_SUGGEST: - case AutocompleteMatch::SEARCH_OTHER_ENGINE: { - static NSImage* searchImage = RetainedResourceImage(IDR_O2_SEARCH); - return searchImage; - } - case AutocompleteMatch::OPEN_HISTORY_PAGE: { - static NSImage* moreImage = RetainedResourceImage(IDR_O2_MORE); - return moreImage; - } - default: - NOTREACHED(); - break; - } - - return nil; -} - } // namespace // Helper for MatchText() to allow sharing code between the contents @@ -386,7 +346,8 @@ void AutocompletePopupViewMac::UpdatePopupAppearance() { for (size_t ii = 0; ii < rows; ++ii) { AutocompleteButtonCell* cell = [matrix cellAtRow:ii column:0]; const AutocompleteMatch& match = model_->result().match_at(ii); - [cell setImage:MatchIcon(match)]; + [cell setImage:RetainedResourceImage(match.starred ? + IDR_O2_STAR : AutocompleteMatch::TypeToIcon(match.type))]; [cell setAttributedTitle:MatchText(match, resultFont, r.size.width)]; } diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc index 044431b..ba33df0 100644 --- a/chrome/browser/autocomplete/search_provider.cc +++ b/chrome/browser/autocomplete/search_provider.cc @@ -98,8 +98,8 @@ void SearchProvider::Start(const AutocompleteInput& input, // User typed "?" alone. Give them a placeholder result indicating what // this syntax does. if (default_provider) { - AutocompleteMatch match(this, 0, false, - AutocompleteMatch::SEARCH_WHAT_YOU_TYPED); + AutocompleteMatch match; + match.provider = this; match.contents.assign(l10n_util::GetString(IDS_EMPTY_KEYWORD_VALUE)); match.contents_class.push_back( ACMatchClassification(0, ACMatchClassification::NONE)); |