[omnibox] Tab switch suggestions improvements
3 small changes:
- i10n-alize hint string
- don't switch to 'this' tab
still TODO: don't suggest 'this' tab
- push |ConvertOpenTabMatches()| to base class and
call from HPQ and HUP
Bug: 780835
Change-Id: I16507d84e5ccedaae81028c49fd47e650ccda4ab
Reviewed-on: https://chromium-review.googlesource.com/755373
Reviewed-by: Mark Pearson <[email protected]>
Reviewed-by: Justin Donnelly <[email protected]>
Commit-Queue: Justin Donnelly <[email protected]>
Cr-Commit-Position: refs/heads/master@{#514833}diff --git a/components/omnibox/browser/history_provider.cc b/components/omnibox/browser/history_provider.cc
index 367d2430..448191d92 100644
--- a/components/omnibox/browser/history_provider.cc
+++ b/components/omnibox/browser/history_provider.cc
@@ -13,6 +13,8 @@
#include "components/omnibox/browser/autocomplete_input.h"
#include "components/omnibox/browser/autocomplete_match.h"
#include "components/omnibox/browser/in_memory_url_index_types.h"
+#include "components/strings/grit/components_strings.h"
+#include "ui/base/l10n/l10n_util.h"
#include "url/url_util.h"
using bookmarks::BookmarkModel;
@@ -102,3 +104,28 @@
return spans;
}
+
+void HistoryProvider::ConvertOpenTabMatches() {
+ for (auto& match : matches_) {
+ // If url is in a tab, change type, update classification.
+ if (client()->IsTabOpenWithURL(match.destination_url)) {
+ match.type = AutocompleteMatchType::TAB_SEARCH;
+ const base::string16 switch_tab_message =
+ l10n_util::GetStringUTF16(IDS_OMNIBOX_TAB_SUGGEST_HINT) +
+ base::UTF8ToUTF16(" - ");
+ match.description = switch_tab_message + match.description;
+ // Add classfication for the prefix.
+ if (match.description_class[0].style != ACMatchClassification::NONE) {
+ match.description_class.insert(
+ match.description_class.begin(),
+ ACMatchClassification(0, ACMatchClassification::NONE));
+ }
+ // Shift the rest.
+ for (auto& classification : match.description_class) {
+ if (classification.offset != 0 ||
+ classification.style != ACMatchClassification::NONE)
+ classification.offset += switch_tab_message.size();
+ }
+ }
+ }
+}
diff --git a/components/omnibox/browser/history_provider.h b/components/omnibox/browser/history_provider.h
index 9cf3414..8e8b654 100644
--- a/components/omnibox/browser/history_provider.h
+++ b/components/omnibox/browser/history_provider.h
@@ -45,6 +45,10 @@
AutocompleteProviderClient* client() { return client_; }
+ // Converts matches whose URL matches a tab's URL to TAB_SEARCH matches.
+ // Fixes up description as well.
+ void ConvertOpenTabMatches();
+
private:
AutocompleteProviderClient* client_;
diff --git a/components/omnibox/browser/history_quick_provider.cc b/components/omnibox/browser/history_quick_provider.cc
index eda184f..e6810d4 100644
--- a/components/omnibox/browser/history_quick_provider.cc
+++ b/components/omnibox/browser/history_quick_provider.cc
@@ -60,12 +60,6 @@
// autocomplete behavior here.
if (in_memory_url_index_) {
DoAutocomplete();
- // TODO(crbug.com/46623): Theoretically, this would be in
- // |DoAutocomplete()| but it's already crowded. If |Do...| were factored
- // (namely, moving the WYT logic to a subroutine) we should move this call
- // back into |Do...|.
- if (base::FeatureList::IsEnabled(omnibox::kOmniboxTabSwitchSuggestions))
- ConvertOpenTabMatches();
}
}
@@ -94,6 +88,8 @@
// Mark this max_match_score as being used.
max_match_score--;
}
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxTabSwitchSuggestions))
+ ConvertOpenTabMatches();
}
int HistoryQuickProvider::FindMaxMatchScore(
@@ -197,31 +193,6 @@
return max_match_score;
}
-void HistoryQuickProvider::ConvertOpenTabMatches() {
- for (auto& match : matches_) {
- // If URL is in a tab, change type, update classification.
- if (client()->IsTabOpenWithURL(match.destination_url)) {
- match.type = AutocompleteMatchType::TAB_SEARCH;
- // TODO(crbug.com/46623): Make i18n constant.
- const base::string16 switch_tab_message =
- base::UTF8ToUTF16("Switch to tab - ");
- match.description = switch_tab_message + match.description;
- // Add classfication for the prefix.
- if (match.description_class[0].style != ACMatchClassification::NONE) {
- match.description_class.insert(
- match.description_class.begin(),
- ACMatchClassification(0, ACMatchClassification::NONE));
- }
- // Shift the rest.
- for (auto& classification : match.description_class) {
- if (classification.offset != 0 ||
- classification.style != ACMatchClassification::NONE)
- classification.offset += switch_tab_message.size();
- }
- }
- }
-}
-
AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
const ScoredHistoryMatch& history_match,
int score) {
diff --git a/components/omnibox/browser/history_quick_provider.h b/components/omnibox/browser/history_quick_provider.h
index ec93a44..2b1f0fcc 100644
--- a/components/omnibox/browser/history_quick_provider.h
+++ b/components/omnibox/browser/history_quick_provider.h
@@ -51,10 +51,6 @@
// it if we believe that there will be a URL-what-you-typed match.
int FindMaxMatchScore(const ScoredHistoryMatches& matches);
- // Converts matches whose URL matches a tab's URL to TAB_SEARCH matches.
- // Fixes up the description as well.
- void ConvertOpenTabMatches();
-
// Creates an AutocompleteMatch from |history_match|, assigning it
// the score |score|.
AutocompleteMatch QuickMatchToACMatch(const ScoredHistoryMatch& history_match,
diff --git a/components/omnibox/browser/history_url_provider.cc b/components/omnibox/browser/history_url_provider.cc
index 96e91400..d925450 100644
--- a/components/omnibox/browser/history_url_provider.cc
+++ b/components/omnibox/browser/history_url_provider.cc
@@ -898,6 +898,8 @@
}
matches_.push_back(HistoryMatchToACMatch(*params, i, relevance));
}
+ if (base::FeatureList::IsEnabled(omnibox::kOmniboxTabSwitchSuggestions))
+ ConvertOpenTabMatches();
}
done_ = true;
diff --git a/components/omnibox_strings.grdp b/components/omnibox_strings.grdp
index 192b592e..68c01eb 100644
--- a/components/omnibox_strings.grdp
+++ b/components/omnibox_strings.grdp
@@ -40,6 +40,9 @@
Search or type URL
</message>
</if>
+ <message name="IDS_OMNIBOX_TAB_SUGGEST_HINT" desc="The text prefixing a suggestion description to say that this suggestion will switch to another tab.">
+ Switch to tab
+ </message>
<message name="IDS_PHYSICAL_WEB_OVERFLOW_DESCRIPTION" desc="The description in the omnibox dropdown indicating that multiple nearby devices are broadcasting URLs.">
Physical Web suggestions
</message>
