[omnibox] Update UI for starter pack expansion.
This CL includes some UI polish for the new "Ask Google" starter pack
engine.
1. The text on lhs of the omnibox in keyword mode now only shoes the
name ("Search Ask Google" -> "Ask Google")
2. The mirror query is no longer shown when in the "Ask Google" mode.
Screenshots:
Before: https://screenshot.googleplex.com/5kUi55iPhTDnY7T.png
After: https://screenshot.googleplex.com/84Y9daeY8PUTbBU.png
Bug: 1521556
Change-Id: I474994a0960c0b6bb014b5d3f5859fd044649c42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5296788
Commit-Queue: Angela Yoeurng <yoangela@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1260902}
diff --git a/chrome/browser/ui/views/location_bar/selected_keyword_view.cc b/chrome/browser/ui/views/location_bar/selected_keyword_view.cc
index 30d8af4..ede999d 100644
--- a/chrome/browser/ui/views/location_bar/selected_keyword_view.cc
+++ b/chrome/browser/ui/views/location_bar/selected_keyword_view.cc
@@ -26,9 +26,10 @@
KeywordLabelNames names;
if (service) {
bool is_extension_keyword = false;
- names.short_name =
- service->GetKeywordShortName(keyword, &is_extension_keyword);
- names.full_name = is_extension_keyword
+ bool is_ask_google_keyword = false;
+ names.short_name = service->GetKeywordShortName(
+ keyword, &is_extension_keyword, &is_ask_google_keyword);
+ names.full_name = (is_extension_keyword || is_ask_google_keyword)
? names.short_name
: l10n_util::GetStringFUTF16(
IDS_OMNIBOX_KEYWORD_TEXT_MD, names.short_name);
diff --git a/chrome/browser/ui/views/omnibox/omnibox_popup_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_popup_view_views.cc
index 983cfda8..3927acc3 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_popup_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_popup_view_views.cc
@@ -320,6 +320,7 @@
? autocomplete_controller->result().GetHeaderForSuggestionGroup(
match.suggestion_group_id.value())
: u"";
+ bool row_hidden = controller()->IsSuggestionHidden(match);
bool group_hidden = match.suggestion_group_id.has_value() &&
controller()->IsSuggestionGroupHidden(
match.suggestion_group_id.value());
@@ -336,7 +337,7 @@
OmniboxResultView* const result_view = row_view->result_view();
result_view->SetMatch(match);
// Set visibility of the result view based on whether the group is hidden.
- result_view->SetVisible(!group_hidden);
+ result_view->SetVisible(!group_hidden && !row_hidden);
const SkBitmap* bitmap = model()->GetPopupRichSuggestionBitmap(i);
if (bitmap) {
diff --git a/components/omnibox/browser/omnibox_controller.cc b/components/omnibox/browser/omnibox_controller.cc
index 7d5925d..a86f001 100644
--- a/components/omnibox/browser/omnibox_controller.cc
+++ b/components/omnibox/browser/omnibox_controller.cc
@@ -10,13 +10,16 @@
#include "components/omnibox/browser/autocomplete_classifier.h"
#include "components/omnibox/browser/autocomplete_controller_emitter.h"
#include "components/omnibox/browser/autocomplete_match.h"
+#include "components/omnibox/browser/autocomplete_match_type.h"
#include "components/omnibox/browser/location_bar_model.h"
#include "components/omnibox/browser/omnibox_client.h"
#include "components/omnibox/browser/omnibox_edit_model.h"
+#include "components/omnibox/browser/omnibox_field_trial.h"
#include "components/omnibox/browser/omnibox_popup_selection.h"
#include "components/omnibox/browser/omnibox_popup_view.h"
#include "components/omnibox/browser/omnibox_prefs.h"
#include "components/omnibox/browser/page_classification_functions.h"
+#include "components/search_engines/template_url_starter_pack_data.h"
#include "ui/gfx/geometry/rect.h"
OmniboxController::OmniboxController(OmniboxView* view,
@@ -167,6 +170,20 @@
suggestion_group_id);
}
+bool OmniboxController::IsSuggestionHidden(
+ const AutocompleteMatch& match) const {
+ if (OmniboxFieldTrial::IsStarterPackExpansionEnabled() &&
+ match.from_keyword) {
+ TemplateURL* turl =
+ match.GetTemplateURL(client_->GetTemplateURLService(), false);
+ if (turl &&
+ turl->starter_pack_id() == TemplateURLStarterPackData::kAskGoogle) {
+ return true;
+ }
+ }
+ return false;
+}
+
bool OmniboxController::IsSuggestionGroupHidden(
omnibox::GroupId suggestion_group_id) const {
PrefService* prefs = client_->GetPrefs();
diff --git a/components/omnibox/browser/omnibox_controller.h b/components/omnibox/browser/omnibox_controller.h
index ab0c47c..cf31a8d 100644
--- a/components/omnibox/browser/omnibox_controller.h
+++ b/components/omnibox/browser/omnibox_controller.h
@@ -9,6 +9,7 @@
#include "base/compiler_specific.h"
#include "components/omnibox/browser/autocomplete_controller.h"
+#include "components/omnibox/browser/autocomplete_match.h"
#include "components/omnibox/browser/omnibox_edit_model.h"
#include "components/prefs/pref_change_registrar.h"
@@ -70,6 +71,11 @@
std::u16string GetHeaderForSuggestionGroup(
omnibox::GroupId suggestion_group_id) const;
+ // Returns whether or not the row for a particular match should be hidden in
+ // the UI. This is currently used to hide suggestions in the 'AskGoogle' scope
+ // when the starter pack expansion feature is enabled.
+ bool IsSuggestionHidden(const AutocompleteMatch& match) const;
+
// Returns whether or not `suggestion_group_id` should be collapsed in the UI.
// This method takes into account both the user's stored prefs as well as
// the server-provided visibility hint. Returns false if `suggestion_group_id`
diff --git a/components/search_engines/template_url_service.cc b/components/search_engines/template_url_service.cc
index 4dc35e8..1d64ca2 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -1354,12 +1354,15 @@
std::u16string TemplateURLService::GetKeywordShortName(
const std::u16string& keyword,
- bool* is_omnibox_api_extension_keyword) const {
+ bool* is_omnibox_api_extension_keyword,
+ bool* is_ask_google_keyword) const {
const TemplateURL* template_url = GetTemplateURLForKeyword(keyword);
// TODO(sky): Once LocationBarView adds a listener to the TemplateURLService
// to track changes to the model, this should become a DCHECK.
if (template_url) {
+ *is_ask_google_keyword = template_url->starter_pack_id() ==
+ TemplateURLStarterPackData::kAskGoogle;
*is_omnibox_api_extension_keyword =
template_url->type() == TemplateURL::OMNIBOX_API_EXTENSION;
return template_url->AdjustedShortNameForLocaleDirection();
diff --git a/components/search_engines/template_url_service.h b/components/search_engines/template_url_service.h
index d71b22cd..36022cf 100644
--- a/components/search_engines/template_url_service.h
+++ b/components/search_engines/template_url_service.h
@@ -477,10 +477,10 @@
// Returns the locale-direction-adjusted short name for the given keyword.
// Also sets the out param to indicate whether the keyword belongs to an
- // Omnibox extension.
- std::u16string GetKeywordShortName(
- const std::u16string& keyword,
- bool* is_omnibox_api_extension_keyword) const;
+ // Omnibox extension or the AskGoogle starter pack engine.
+ std::u16string GetKeywordShortName(const std::u16string& keyword,
+ bool* is_omnibox_api_extension_keyword,
+ bool* is_ask_google_keyword) const;
// Called by the history service when a URL is visited.
void OnHistoryURLVisited(const URLVisitedDetails& details);