Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(247)

Unified Diff: components/search_engines/template_url.cc

Issue 1238683003: Unpunycode search keywords and short names. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@lkgr
Patch Set: Restrict IDN-decoding to keywords generated from URL; Use prefs::kAcceptLanguages for IDN-decoding Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/search_engines/template_url.cc
diff --git a/components/search_engines/template_url.cc b/components/search_engines/template_url.cc
index dbb74fc77853e1fe3c15533b7a1740a360ecf6fe..1862e5a974b47c5845a9d7def3c6be56db6f6ce6 100644
--- a/components/search_engines/template_url.cc
+++ b/components/search_engines/template_url.cc
@@ -1210,14 +1210,19 @@ TemplateURL::~TemplateURL() {
}
// static
-base::string16 TemplateURL::GenerateKeyword(const GURL& url) {
+base::string16 TemplateURL::GenerateKeyword(
+ const GURL& url,
+ const std::string& accept_languages) {
DCHECK(url.is_valid());
+ // Search engine's keyword is a user facing value and therefore
+ // should be IDN-decoded using user's accepted languages.
Peter Kasting 2015/07/20 19:30:52 This rationale seems a bit inaccurate. How about
alshabalin 2015/07/21 08:44:19 Done. But I worded it like this: // Since keywo
+ base::string16 keyword = net::IDNToUnicode(url.host(), accept_languages);
Peter Kasting 2015/07/20 19:30:52 Nit: Combine this with the line below.
// Strip "www." off the front of the keyword; otherwise the keyword won't work
// properly. See http://code.google.com/p/chromium/issues/detail?id=6984 .
// Special case: if the host was exactly "www." (not sure this can happen but
Peter Kasting 2015/07/20 19:30:52 Nit: Move this "Special case" comment below the St
// perhaps with some weird intranet and custom DNS server?), ensure we at
// least don't return the empty string.
- base::string16 keyword(net::StripWWWFromHost(url));
+ keyword = net::StripWWW(keyword);
return keyword.empty() ? base::ASCIIToUTF16("www") : keyword;
}
@@ -1485,7 +1490,8 @@ void TemplateURL::ResetKeywordIfNecessary(
DCHECK(GetType() != OMNIBOX_API_EXTENSION);
GURL url(GenerateSearchURL(search_terms_data));
if (url.is_valid())
- data_.SetKeyword(GenerateKeyword(url));
+ data_.SetKeyword(
+ GenerateKeyword(url, search_terms_data.GetAcceptedLanguages()));
}
}

Powered by Google App Engine
This is Rietveld 408576698