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

chrome/browser/history/query_parser.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/history/query_parser.h"
 
 #include <algorithm>
 
 #include "base/compiler_specific.h"
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
+#include "base/string_util.h"
 
 namespace {
 
 // Returns true if |mp1.first| is less than |mp2.first|. This is used to
 // sort match positions.
 int CompareMatchPosition(const Snippet::MatchPosition& mp1,
                          const Snippet::MatchPosition& mp2) {
   return mp1.first < mp2.first;
 }
 
@@ skipping 15 matching lines @@
       mp.second = i->second;
       i = matches->erase(i);
     } else {
       return;
     }
   }
 }
 
 // Sorts the match positions in |matches| by their first index, then coalesces
 // any match positions that intersect each other.
-void CoalseAndSortMatchPositions(Snippet::MatchPositions* matches) {
+void CoalesceAndSortMatchPositions(Snippet::MatchPositions* matches) {
   std::sort(matches->begin(), matches->end(), &CompareMatchPosition);
   // WARNING: we don't use iterator here as CoalesceMatchesFrom may remove
   // from matches.
   for (size_t i = 0; i < matches->size(); ++i)
     CoalesceMatchesFrom(i, matches);
 }
 
 // Returns true if the character is considered a quote.
 bool IsQueryQuote(wchar_t ch) {
   return ch == '"' ||
          ch == 0xab ||    // left pointing double angle bracket
          ch == 0xbb ||    // right pointing double angle bracket
          ch == 0x201c ||  // left double quotation mark
          ch == 0x201d ||  // right double quotation mark
          ch == 0x201e;    // double low-9 quotation mark
 }
 
+// Returns true if the character is considered non-breaking when it appears in
+// the middle of a word. This can be used to prevent an URL-like query from
+// being broken into multiple words.
+bool IsNonBreakingSymbol(wchar_t ch) {
+  return ch == '.' ||
+         ch == '-' ||
+         ch == '_' ||
+         ch == '/' ||
+         ch == '~';
+}
+
 }  // namespace
 
 // Inheritance structure:
 // Queries are represented as trees of QueryNodes.
 // QueryNodes are either a collection of subnodes (a QueryNodeList)
 // or a single word (a QueryNodeWord).
 
 // A QueryNodeWord is a single word in the query.

[sky, 2012/02/08 23:31:28]

Update description.

[Patrick Dubroy, 2012/02/09 14:47:00]

Done.

 class QueryNodeWord : public QueryNode {
  public:
   explicit QueryNodeWord(const string16& word);
   virtual ~QueryNodeWord();
 
   const string16& word() const { return word_; }
 
   void set_literal(bool literal) { literal_ = literal; }
 
   // QueryNode:
   virtual int AppendToSQLiteQuery(string16* query) const OVERRIDE;
   virtual bool IsWord() const OVERRIDE;
   virtual bool Matches(const string16& word, bool exact) const OVERRIDE;
   virtual bool HasMatchIn(
       const std::vector<QueryWord>& words,
       Snippet::MatchPositions* match_positions) const OVERRIDE;
   virtual void AppendWords(std::vector<string16>* words) const OVERRIDE;
+  virtual void Append(const string16& word);

[sky, 2012/02/08 23:31:28]

This isn't part of QueryNode, so move it some where else. Additionally does it
really need to be virtual?

[Patrick Dubroy, 2012/02/09 14:47:00]

Whoops, yeah. Done.

 
  private:
   string16 word_;
   bool literal_;
 
   DISALLOW_COPY_AND_ASSIGN(QueryNodeWord);
 };
 
 QueryNodeWord::QueryNodeWord(const string16& word)
     : word_(word),
@@ skipping 32 matching lines @@
       return true;
     }
   }
   return false;
 }
 
 void QueryNodeWord::AppendWords(std::vector<string16>* words) const {
   words->push_back(word_);
 }
 
+void QueryNodeWord::Append(const string16& str) {
+  word_ += str;
+}
+
 // A QueryNodeList has a collection of QueryNodes which are deleted in the end.
 class QueryNodeList : public QueryNode {
  public:
   typedef std::vector<QueryNode*> QueryNodeVector;
 
   QueryNodeList();
   virtual ~QueryNodeList();
 
   QueryNodeVector* children() { return &children_; }
 
@@ skipping 186 matching lines @@
     if (!query_nodes[i]->HasMatchIn(query_words, &matches))
       return false;
   }
   if (lower_text.length() != text.length()) {
     // The lower case string differs from the original string. The matches are
     // meaningless.
     // TODO(sky): we need a better way to align the positions so that we don't
     // completely punt here.
     match_positions->clear();
   } else {
-    CoalseAndSortMatchPositions(&matches);
+    CoalesceAndSortMatchPositions(&matches);
     match_positions->swap(matches);
   }
   return true;
 }
 
 bool QueryParser::ParseQueryImpl(const string16& query, QueryNodeList* root) {
   base::i18n::BreakIterator iter(query, base::i18n::BreakIterator::BREAK_WORD);
   // TODO(evanm): support a locale here
   if (!iter.Init())
     return false;
 
   // To handle nesting, we maintain a stack of QueryNodeLists.
   // The last element (back) of the stack contains the current, deepest node.
   std::vector<QueryNodeList*> query_stack;
   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;
-        }
+    // is not necessarily a word, it could also be a punctuation or whitespace
+    // character. Punctuation is preserved inside quotes, and otherwise removed
+    // except if it is a non-breaking character in the middle of a word.
+
+    wchar_t last_char = query[iter.prev()];
+    if (IsQueryQuote(last_char)) {
+      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 && !IsWhitespace(last_char))) {
+      // Append to the current word if the new token is a word or a non-
+      // whitespace character inside quotes.
+      if (current_word) {
+        current_word->Append(iter.GetString());
+      } else {
+        current_word = new QueryNodeWord(iter.GetString());
+        current_word->set_literal(in_quotes);
+        query_stack.back()->AddChild(current_word);
+      }
+    } else if (current_word != NULL) {
+      // Allow non-breaking symbols inside a word.
+      // Any other punctuation or whitespace character ends the current word.
+      if (IsNonBreakingSymbol(last_char))
+        current_word->Append(iter.GetString());
+      else
+        current_word = NULL;
     }
   }
 
   root->RemoveEmptySubnodes();
   return true;
 }
 
 void QueryParser::ExtractQueryWords(const string16& text,
                                     std::vector<QueryWord>* words) {
   base::i18n::BreakIterator iter(text, base::i18n::BreakIterator::BREAK_WORD);
   // TODO(evanm): support a locale here
   if (!iter.Init())
     return;
 
   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()) {
       string16 word = iter.GetString();
       if (!word.empty()) {
         words->push_back(QueryWord());
         words->back().word = word;
         words->back().position = iter.prev();
       }
     }
   }
 }