The search terms are escaped using + or %20 for space depending on whether replacement is in query part of the URL or not.

Removed duplicate EscapeQueryParamValue functions without |use_plus| argument.

BUG=24571
TEST=Verify that space is escaped as stated in description; see bug description for example with search on Wikipedia.
Review URL: http://codereview.chromium.org/543077

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36398 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 42 insertions, 13 deletions

diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc
index 82bec3b..26da6fa 100644
--- a/chrome/browser/search_engines/template_url.cc
+++ b/chrome/browser/search_engines/template_url.cc
@@ -246,6 +246,19 @@ std::wstring TemplateURLRef::ReplaceSearchTerms(
   if (replacements_.empty())
     return parsed_url_;
 
+  // Determine if the search terms are in the query or before. We're escaping
+  // space as '+' in the former case and as '%20' in the latter case.
+  bool use_plus = true;
+  for (Replacements::iterator i = replacements_.begin();
+       i != replacements_.end(); ++i) {
+    if (i->type == SEARCH_TERMS) {
+      std::wstring::size_type query_start = parsed_url_.find(L'?');
+      use_plus = query_start != std::wstring::npos &&
+          (static_cast<std::wstring::size_type>(i->index) > query_start);
+      break;
+    }
+  }
+
   // Encode the search terms so that we know the encoding.
   const std::vector<std::string>& encodings = host.input_encodings();
   string16 encoded_terms;
@@ -253,21 +266,23 @@ std::wstring TemplateURLRef::ReplaceSearchTerms(
   std::wstring input_encoding;
   for (size_t i = 0; i < encodings.size(); ++i) {
     if (EscapeQueryParamValue(WideToUTF16Hack(terms),
-                              encodings[i].c_str(), &encoded_terms)) {
+                              encodings[i].c_str(), use_plus, &encoded_terms)) {
       if (!original_query_for_suggestion.empty()) {
         EscapeQueryParamValue(WideToUTF16Hack(original_query_for_suggestion),
-                              encodings[i].c_str(), &encoded_original_query);
+                              encodings[i].c_str(),
+                              true,
+                              &encoded_original_query);
       }
       input_encoding = ASCIIToWide(encodings[i]);
       break;
     }
   }
   if (input_encoding.empty()) {
-    encoded_terms = WideToUTF16Hack(EscapeQueryParamValueUTF8(terms));
+    encoded_terms = WideToUTF16Hack(EscapeQueryParamValueUTF8(terms, use_plus));
     if (!original_query_for_suggestion.empty()) {
       encoded_original_query =
           WideToUTF16Hack(
-              EscapeQueryParamValueUTF8(original_query_for_suggestion));
+              EscapeQueryParamValueUTF8(original_query_for_suggestion, true));
     }
     input_encoding = L"UTF-8";
   }
diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc
index f15d6fc3..261adf0 100644
--- a/chrome/browser/search_engines/template_url_unittest.cc
+++ b/chrome/browser/search_engines/template_url_unittest.cc
@@ -38,15 +38,29 @@ TEST_F(TemplateURLTest, TestValidWithComplete) {
 }
 
 TEST_F(TemplateURLTest, URLRefTestSearchTerms) {
-  TemplateURL t_url;
-  TemplateURLRef ref(L"http://foo{searchTerms}", 0, 0);
-  ASSERT_TRUE(ref.IsValid());
-
-  ASSERT_TRUE(ref.SupportsReplacement());
-  GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, L"search",
-      TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
-  ASSERT_TRUE(result.is_valid());
-  ASSERT_EQ("http://foosearch/", result.spec());
+  struct SearchTermsCase {
+    const wchar_t* url;
+    const wchar_t* terms;
+    const char* output;
+  } search_term_cases[] = {
+    { L"http://foo{searchTerms}", L"sea rch", "http://foosea%20rch/" },
+    { L"http://foo{searchTerms}?boo=abc", L"sea rch",
+      "http://foosea%20rch/?boo=abc" },
+    { L"http://foo/?boo={searchTerms}", L"sea rch",
+      "http://foo/?boo=sea+rch" }
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(search_term_cases); ++i) {
+    const SearchTermsCase& value = search_term_cases[i];
+    TemplateURL t_url;
+    TemplateURLRef ref(value.url, 0, 0);
+    ASSERT_TRUE(ref.IsValid());
+
+    ASSERT_TRUE(ref.SupportsReplacement());
+    GURL result = GURL(WideToUTF8(ref.ReplaceSearchTerms(t_url, value.terms,
+        TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, std::wstring())));
+    ASSERT_TRUE(result.is_valid());
+    ASSERT_EQ(value.output, result.spec());
+  }
 }
 
 TEST_F(TemplateURLTest, URLRefTestCount) {