Handle typographical apostrophe with spelling service client

chrome/renderer/spellchecker/spellcheck.cc

 // Copyright (c) 2012 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/renderer/spellchecker/spellcheck.h"
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 79 matching lines @@
   std::transform(words.begin(), words.end(), words_.begin(),
                  [](const std::string& w) { return WebString::fromUTF8(w); });
 }
 
 bool DocumentMarkersRemover::Visit(content::RenderView* render_view) {
   if (render_view && render_view->GetWebView())
     render_view->GetWebView()->removeSpellingMarkersUnderWords(words_);
   return true;
 }
 
+bool IsApostrophe(wchar_t c) {

[groby-ooo-7-16, 2015/06/18 21:52:26]

This still doesn't work - the for-loop in Preserve... uses char16. wchar_t and
char16 are not necessarily convertible. (Again, I don't know if that's my
paranoia or not - I'd ask brettw around this)

Personally, I'd take in a char16, and have two constants

const base::char16 kRightSingleQuotationMark  = 0x2019
const base::char16 kApostrophe = 0x27;

and then compare against these, which avoids all wchar_t nonsense

[please use gerrit instead, 2015/06/19 01:37:00]

Done.

+  return c == L'\'' || c == L'\x2019';
+}
+
+// Makes sure that the apostrophes in the |spelling_suggestion| are the same
+// type as in the |misspelled_word| and in the same order. Ignore differences in
+// the number of apostrophes.
+void PreserveOriginalApostropheTypes(const base::string16& misspelled_word,
+                                     base::string16* spelling_suggestion) {
+  auto it = spelling_suggestion->begin();
+  for (const base::char16& c : misspelled_word) {
+    if (IsApostrophe(c)) {
+      it = std::find_if(it, spelling_suggestion->end(), IsApostrophe);
+      if (it == spelling_suggestion->end())
+        return;
+
+      *it++ = c;
+    }
+  }
+}
+
 }  // namespace
 
 class SpellCheck::SpellcheckRequest {
  public:
   SpellcheckRequest(const base::string16& text,
                     blink::WebTextCheckingCompletion* completion)
       : text_(text), completion_(completion) {
     DCHECK(completion);
   }
   ~SpellcheckRequest() {}
@@ skipping 266 matching lines @@
   }
 }
 #endif
 
 void SpellCheck::CreateTextCheckingResults(
     ResultFilter filter,
     int line_offset,
     const base::string16& line_text,
     const std::vector<SpellCheckResult>& spellcheck_results,
     WebVector<WebTextCheckingResult>* textcheck_results) {
-  // Double-check misspelled words with our spellchecker and attach grammar
-  // markers to them if our spellchecker tells they are correct words, i.e. they
-  // are probably contextually-misspelled words.

[groby-ooo-7-16, 2015/06/18 21:52:26]

Why are we removing this comment? It's making at least somewhat clear what we do
here :)

[please use gerrit instead, 2015/06/19 01:37:00]

There're three things happening in this function now, but this comment is
talking about only one. I moved this comment down to line 426 and "trimmed" it a
bit in the process. I can see how that can get confusing. I've copied the
comment verbatim now.

-  const base::char16* text = line_text.c_str();
-  std::vector<WebTextCheckingResult> list;
-  for (size_t i = 0; i < spellcheck_results.size(); ++i) {
-    SpellCheckResult::Decoration decoration = spellcheck_results[i].decoration;
-    int word_location = spellcheck_results[i].location;
-    int word_length = spellcheck_results[i].length;
-    int misspelling_start = 0;
-    int misspelling_length = 0;
+  std::vector<WebTextCheckingResult> results;
+  for (const SpellCheckResult& spellcheck_result : spellcheck_results) {
+    const base::string16& misspelled_word =
+        line_text.substr(spellcheck_result.location, spellcheck_result.length);
+
+    // Ignore words in custom dictionary.
+    if (custom_dictionary_.SpellCheckWord(misspelled_word, 0,
+                                          misspelled_word.length())) {
+      continue;
+    }
+
+    // Use the same types of appostrophes as in the mispelled word.
+    base::string16 replacement = spellcheck_result.replacement;
+    PreserveOriginalApostropheTypes(misspelled_word, &replacement);
+
+    // Ignore misspellings due the typographical apostrophe.
+    if (misspelled_word == replacement)
+      continue;
+
+    // Use grammar decoration when the server spellcheck marks a word
+    // misspelled, but the local spellcheck does not.
+    SpellCheckResult::Decoration decoration = spellcheck_result.decoration;
+    int unused_misspelling_start = 0;
+    int unused_misspelling_length = 0;
     if (decoration == SpellCheckResult::SPELLING &&
-        filter == USE_NATIVE_CHECKER) {
-      if (SpellCheckWord(text + word_location, word_length, 0,
-                         &misspelling_start, &misspelling_length, NULL)) {
-        decoration = SpellCheckResult::GRAMMAR;
-      }
+        filter == USE_NATIVE_CHECKER &&
+        SpellCheckWord(misspelled_word.c_str(), misspelled_word.length(), 0,
+                       &unused_misspelling_start, &unused_misspelling_length,
+                       nullptr)) {
+      decoration = SpellCheckResult::GRAMMAR;
     }
-    if (!custom_dictionary_.SpellCheckWord(
-            line_text, word_location, word_length)) {
-      list.push_back(WebTextCheckingResult(
-          static_cast<WebTextDecorationType>(decoration),
-          word_location + line_offset,
-          word_length,
-          spellcheck_results[i].replacement,
-          spellcheck_results[i].hash));
-    }
+
+    results.push_back(WebTextCheckingResult(
+        static_cast<WebTextDecorationType>(decoration),
+        line_offset + spellcheck_result.location, spellcheck_result.length,
+        replacement, spellcheck_result.hash));
   }
-  textcheck_results->assign(list);
+
+  textcheck_results->assign(results);
 }