Don't strip punctuation inside quotes in history search queries.

chrome/browser/history/query_parser.cc

Index: chrome/browser/history/query_parser.cc
diff --git a/chrome/browser/history/query_parser.cc b/chrome/browser/history/query_parser.cc
index 782c708010cf2b9b8ef0ed7216f6740a95c8b4aa..181dc2135a04345d4ec534ecc3ed0b843ddd0fbc 100644
--- a/chrome/browser/history/query_parser.cc
+++ b/chrome/browser/history/query_parser.cc
@@ -80,6 +80,7 @@ class QueryNodeWord : public QueryNode {
   const string16& word() const { return word_; }
 
   void set_literal(bool literal) { literal_ = literal; }
+  void AppendText(const string16& text) { word_ += text; }

[mrossetti, 2012/02/07 01:05:57]

We generally only inline simple getters and setters. This should not be inlined.

 
   // QueryNode:
   virtual int AppendToSQLiteQuery(string16* query) const OVERRIDE;
@@ -365,27 +366,32 @@ bool QueryParser::ParseQueryImpl(const string16& query, QueryNodeList* root) {
   query_stack.push_back(root);
 
   bool in_quotes = false;  // whether we're currently in a quoted phrase
+  QueryNodeWord* current_word = NULL;
   while (iter.Advance()) {
     // Just found a span between 'prev' (inclusive) and 'pos' (exclusive). It
     // is not necessarily a word, but could also be a sequence of punctuation
     // or whitespace.
-    if (iter.IsWord()) {
-      QueryNodeWord* word_node = new QueryNodeWord(iter.GetString());
-      if (in_quotes)
-        word_node->set_literal(true);
-      query_stack.back()->AddChild(word_node);
-    } else {  // Punctuation.
-      if (IsQueryQuote(query[iter.prev()])) {
-        if (!in_quotes) {
-          QueryNodeList* quotes_node = new QueryNodePhrase;
-          query_stack.back()->AddChild(quotes_node);
-          query_stack.push_back(quotes_node);
-          in_quotes = true;
-        } else {
-          query_stack.pop_back();  // Stop adding to the quoted phrase.
-          in_quotes = false;
-        }
+    if (IsQueryQuote(query[iter.prev()])) {
+      if (!in_quotes) {
+        QueryNodeList* quotes_node = new QueryNodePhrase;
+        query_stack.back()->AddChild(quotes_node);
+        query_stack.push_back(quotes_node);
+        in_quotes = true;
+      } else {
+        query_stack.pop_back();  // Stop adding to the quoted phrase.
+        in_quotes = false;
       }
+      current_word = NULL;
+    } else if (iter.IsWord() || in_quotes) {

[mrossetti, 2012/02/07 01:05:57]

Okay, as you can see from the unit tests, trying to handle both an open quote
run AND just a regular word here (at line 385) causes the parser results for
string containing special characters to be different than with the current code.

I suggest only handling the |in_quotes| case on this branch, and handling the
unquoted words down below in those empty braces starting on line 394.

+      if (current_word) {
+        current_word->AppendText(iter.GetString());
+      } else {
+        current_word = new QueryNodeWord(iter.GetString());
+        query_stack.back()->AddChild(current_word);

[mrossetti, 2012/02/07 01:05:57]

Do the set_literal here...

+      }
+      if (in_quotes)

[mrossetti, 2012/02/07 01:05:57]

...not here.

+        current_word->set_literal(true);
+    } else {  // Punctuation or whitespace outside quotes.

[mrossetti, 2012/02/07 01:05:57]

These empty braces are just screaming for something! ;^) I believe if you change
this to:

    } else if (iter.IsWord()) {
      QueryNodeWord* word_node = new QueryNodeWord(iter.GetString());
      query_stack.back()->AddChild(word_node);
    }

it will restore the old, unquoted functionality and please the unit tests.

     }
   }