Commit 4a57957a authored by Kevin Bailey's avatar Kevin Bailey Committed by Commit Bot

[omnibox] Don't add WYT search in keyword mode

When in keyword mode, results will be from that search provider,
except for a single WYT search at the bottom. It looks odd to
have a different search there, when the user asked for a keyword
search. This CL doesn't add the match in that case.

Bug: 837395
Change-Id: I1441a6fe07f5d1e7ac5cd3dc16de408df6ad18c9
Reviewed-on: https://chromium-review.googlesource.com/1030613Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554762}
parent ca5d3397
...@@ -98,7 +98,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DeleteOmniboxSuggestionResult) { ...@@ -98,7 +98,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DeleteOmniboxSuggestionResult) {
// Peek into the controller to see if it has the results we expect. // Peek into the controller to see if it has the results we expect.
const AutocompleteResult& result = autocomplete_controller->result(); const AutocompleteResult& result = autocomplete_controller->result();
ASSERT_EQ(4U, result.size()) << AutocompleteResultAsString(result); ASSERT_EQ(3U, result.size()) << AutocompleteResultAsString(result);
EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(0).fill_into_edit); EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(0).fill_into_edit);
EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD, EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD,
...@@ -117,11 +117,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DeleteOmniboxSuggestionResult) { ...@@ -117,11 +117,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DeleteOmniboxSuggestionResult) {
// Verify that the second omnibox extension suggestion is not deletable. // Verify that the second omnibox extension suggestion is not deletable.
EXPECT_FALSE(result.match_at(2).deletable); EXPECT_FALSE(result.match_at(2).deletable);
EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(3).fill_into_edit);
EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
result.match_at(3).type);
EXPECT_FALSE(result.match_at(3).deletable);
// This test portion is excluded from Mac because the Mac key combination // This test portion is excluded from Mac because the Mac key combination
// FN+SHIFT+DEL used to delete an omnibox suggestion cannot be reproduced. // FN+SHIFT+DEL used to delete an omnibox suggestion cannot be reproduced.
// This is because the FN key is not supported in interactive_test_util.h. // This is because the FN key is not supported in interactive_test_util.h.
...@@ -140,10 +135,9 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DeleteOmniboxSuggestionResult) { ...@@ -140,10 +135,9 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, DeleteOmniboxSuggestionResult) {
// Verify that the first suggestion result was deleted. There should be one // Verify that the first suggestion result was deleted. There should be one
// less suggestion result, 3 now instead of 4. // less suggestion result, 3 now instead of 4.
ASSERT_EQ(3U, result.size()); ASSERT_EQ(2U, result.size());
EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(0).fill_into_edit); EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(0).fill_into_edit);
EXPECT_EQ(base::ASCIIToUTF16("kw n2"), result.match_at(1).fill_into_edit); EXPECT_EQ(base::ASCIIToUTF16("kw n2"), result.match_at(1).fill_into_edit);
EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(2).fill_into_edit);
#endif #endif
} }
...@@ -171,7 +165,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, ExtensionSuggestionsOnlyInKeywordMode) { ...@@ -171,7 +165,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, ExtensionSuggestionsOnlyInKeywordMode) {
// Peek into the controller to see if it has the results we expect. // Peek into the controller to see if it has the results we expect.
{ {
const AutocompleteResult& result = autocomplete_controller->result(); const AutocompleteResult& result = autocomplete_controller->result();
ASSERT_EQ(4U, result.size()) << AutocompleteResultAsString(result); ASSERT_EQ(3U, result.size()) << AutocompleteResultAsString(result);
EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(0).fill_into_edit); EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(0).fill_into_edit);
EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD, EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD,
...@@ -184,10 +178,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, ExtensionSuggestionsOnlyInKeywordMode) { ...@@ -184,10 +178,6 @@ IN_PROC_BROWSER_TEST_F(OmniboxApiTest, ExtensionSuggestionsOnlyInKeywordMode) {
EXPECT_EQ(base::ASCIIToUTF16("kw n2"), result.match_at(2).fill_into_edit); EXPECT_EQ(base::ASCIIToUTF16("kw n2"), result.match_at(2).fill_into_edit);
EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD, EXPECT_EQ(AutocompleteProvider::TYPE_KEYWORD,
result.match_at(2).provider->type()); result.match_at(2).provider->type());
EXPECT_EQ(base::ASCIIToUTF16("kw d"), result.match_at(3).fill_into_edit);
EXPECT_EQ(AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
result.match_at(3).type);
} }
// Now clear the omnibox by pressing escape multiple times, focus the // Now clear the omnibox by pressing escape multiple times, focus the
......
...@@ -594,11 +594,9 @@ TEST_F(AutocompleteProviderTest, ExtraQueryParams) { ...@@ -594,11 +594,9 @@ TEST_F(AutocompleteProviderTest, ExtraQueryParams) {
switches::kExtraSearchQueryParams, "a=b"); switches::kExtraSearchQueryParams, "a=b");
RunExactKeymatchTest(true); RunExactKeymatchTest(true);
CopyResults(); CopyResults();
ASSERT_EQ(2U, result_.size()); ASSERT_EQ(1U, result_.size());
EXPECT_EQ("http://keyword/test", EXPECT_EQ("http://keyword/test",
result_.match_at(0)->destination_url.possibly_invalid_spec()); result_.match_at(0)->destination_url.possibly_invalid_spec());
EXPECT_EQ("http://defaultturl/k%20test?a=b",
result_.match_at(1)->destination_url.possibly_invalid_spec());
} }
// Test that redundant associated keywords are removed. // Test that redundant associated keywords are removed.
......
...@@ -995,22 +995,25 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() { ...@@ -995,22 +995,25 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
} }
} }
SearchSuggestionParser::SuggestResult verbatim( // Don't add WYT when user requested keyword search.
/*suggestion=*/trimmed_verbatim, if (keyword_input_.text().empty()) {
AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED, SearchSuggestionParser::SuggestResult verbatim(
/*subtype_identifier=*/0, /*suggestion=*/trimmed_verbatim,
/*match_contents=*/trimmed_verbatim, AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED,
/*match_contents_prefix=*/base::string16(), /*subtype_identifier=*/0,
/*annotation=*/base::string16(), answer_contents, answer_type, /*match_contents=*/trimmed_verbatim,
std::move(answer), /*suggest_query_params=*/std::string(), /*match_contents_prefix=*/base::string16(),
/*deletion_url=*/std::string(), /*annotation=*/base::string16(), answer_contents, answer_type,
/*image_dominant_color=*/std::string(), std::move(answer), /*suggest_query_params=*/std::string(),
/*image_url=*/std::string(), /*deletion_url=*/std::string(),
/*from_keyword_provider=*/false, verbatim_relevance, /*image_dominant_color=*/std::string(),
relevance_from_server, /*should_prefetch=*/false, /*image_url=*/std::string(),
/*input_text=*/trimmed_verbatim); /*from_keyword_provider=*/false, verbatim_relevance,
AddMatchToMap(verbatim, std::string(), did_not_accept_default_suggestion, relevance_from_server, /*should_prefetch=*/false,
false, keyword_url != nullptr, &map); /*input_text=*/trimmed_verbatim);
AddMatchToMap(verbatim, std::string(), did_not_accept_default_suggestion,
false, keyword_url != nullptr, &map);
}
} }
if (!keyword_input_.text().empty()) { if (!keyword_input_.text().empty()) {
// We only create the verbatim search query match for a keyword // We only create the verbatim search query match for a keyword
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment