fix: contractions handling in spellchecker (#18506)

This fixes #18459 by improving the handling of contractions in the spellcheck API. Specifically, it now accepts contraction words where the spellchecker recognizes the whole word, and not, as previously, just if it recognizes all of its parts.

atom/renderer/api/atom_api_spell_check_client.cc

@@ -5,6 +5,8 @@
 #include "atom/renderer/api/atom_api_spell_check_client.h"
 
 #include <map>
+#include <set>
+#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -39,14 +41,16 @@ bool HasWordCharacters(const base::string16& text, int index) {
   return false;
 }
 
+struct Word {
+  blink::WebTextCheckingResult result;
+  base::string16 text;
+  std::vector<base::string16> contraction_words;
+};
+
 }  // namespace
 
 class SpellCheckClient::SpellcheckRequest {
  public:
-  // Map of individual words to list of occurrences in text
-  using WordMap =
-      std::map<base::string16, std::vector<blink::WebTextCheckingResult>>;
-
   SpellcheckRequest(
       const base::string16& text,
       std::unique_ptr<blink::WebTextCheckingCompletion> completion)
@@ -55,11 +59,11 @@ class SpellCheckClient::SpellcheckRequest {
 
   const base::string16& text() const { return text_; }
   blink::WebTextCheckingCompletion* completion() { return completion_.get(); }
-  WordMap& wordmap() { return word_map_; }
+  std::vector<Word>& wordlist() { return word_list_; }
 
  private:
-  base::string16 text_;  // Text to be checked in this task.
-  WordMap word_map_;     // WordMap to hold distinct words in text
+  base::string16 text_;          // Text to be checked in this task.
+  std::vector<Word> word_list_;  // List of Words found in text
   // The interface to send the misspelled ranges to WebKit.
   std::unique_ptr<blink::WebTextCheckingCompletion> completion_;
 
@@ -150,9 +154,9 @@ void SpellCheckClient::SpellCheckText() {
   base::string16 word;
   size_t word_start;
   size_t word_length;
-  std::vector<base::string16> words;
-  auto& word_map = pending_request_param_->wordmap();
-  blink::WebTextCheckingResult result;
+  std::set<base::string16> words;
+  auto& word_list = pending_request_param_->wordlist();
+  Word word_entry;
   for (;;) {  // Run until end of text
     const auto status =
         text_iterator_.GetNextWord(&word, &word_start, &word_length);
@@ -161,23 +165,18 @@ void SpellCheckClient::SpellCheckText() {
     if (status == SpellcheckWordIterator::IS_SKIPPABLE)
       continue;
 
-    result.location = base::checked_cast<int>(word_start);
-    result.length = base::checked_cast<int>(word_length);
+    word_entry.result.location = base::checked_cast<int>(word_start);
+    word_entry.result.length = base::checked_cast<int>(word_length);
+    word_entry.text = word;
+    word_entry.contraction_words.clear();
 
+    word_list.push_back(word_entry);
+    words.insert(word);
     // If the given word is a concatenated word of two or more valid words
     // (e.g. "hello:hello"), we should treat it as a valid word.
-    std::vector<base::string16> contraction_words;
-    if (!IsContraction(scope, word, &contraction_words)) {
-      words.push_back(word);
-      word_map[word].push_back(result);
-    } else {
-      // For a contraction, we want check the spellings of each individual
-      // part, but mark the entire word incorrect if any part is misspelled
-      // Hence, we use the same word_start and word_length values for every
-      // part of the contraction.
-      for (const auto& w : contraction_words) {
-        words.push_back(w);
-        word_map[w].push_back(result);
+    if (IsContraction(scope, word, &word_entry.contraction_words)) {
+      for (const auto& w : word_entry.contraction_words) {
+        words.insert(w);
       }
     }
   }
@@ -189,29 +188,35 @@ void SpellCheckClient::SpellCheckText() {
 void SpellCheckClient::OnSpellCheckDone(
     const std::vector<base::string16>& misspelled_words) {
   std::vector<blink::WebTextCheckingResult> results;
-
-  auto& word_map = pending_request_param_->wordmap();
-
-  // Take each word from the list of misspelled words received, find their
-  // corresponding WebTextCheckingResult that's stored in the map and pass
-  // all the results to blink through the completion callback.
-  for (const auto& word : misspelled_words) {
-    auto iter = word_map.find(word);
-    if (iter != word_map.end()) {
-      // Word found in map, now gather all the occurrences of the word
-      // from the map value
-      auto& words = iter->second;
-      results.insert(results.end(), words.begin(), words.end());
-      words.clear();
+  std::unordered_set<base::string16> misspelled(misspelled_words.begin(),
+                                                misspelled_words.end());
+
+  auto& word_list = pending_request_param_->wordlist();
+
+  for (const auto& word : word_list) {
+    if (misspelled.find(word.text) != misspelled.end()) {
+      // If this is a contraction, iterate through parts and accept the word
+      // if none of them are misspelled
+      if (!word.contraction_words.empty()) {
+        auto all_correct = true;
+        for (const auto& contraction_word : word.contraction_words) {
+          if (misspelled.find(contraction_word) != misspelled.end()) {
+            all_correct = false;
+            break;
+          }
+        }
+        if (all_correct)
+          continue;
+      }
+      results.push_back(word.result);
     }
   }
   pending_request_param_->completion()->DidFinishCheckingText(results);
   pending_request_param_ = nullptr;
 }
 
-void SpellCheckClient::SpellCheckWords(
-    const SpellCheckScope& scope,
-    const std::vector<base::string16>& words) {
+void SpellCheckClient::SpellCheckWords(const SpellCheckScope& scope,
+                                       const std::set<base::string16>& words) {
   DCHECK(!scope.spell_check_.IsEmpty());
 
   v8::Local<v8::FunctionTemplate> templ = mate::CreateFunctionTemplate(

atom/renderer/api/atom_api_spell_check_client.h

@@ -6,6 +6,7 @@
 #define ATOM_RENDERER_API_ATOM_API_SPELL_CHECK_CLIENT_H_
 
 #include <memory>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -68,7 +69,7 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient,
   // The javascript function will callback OnSpellCheckDone
   // with the results of all the misspelled words.
   void SpellCheckWords(const SpellCheckScope& scope,
-                       const std::vector<base::string16>& words);
+                       const std::set<base::string16>& words);
 
   // Returns whether or not the given word is a contraction of valid words
   // (e.g. "word:word").

spec/api-web-frame-spec.js

@@ -40,19 +40,19 @@ describe('webFrame module', function () {
     const spellCheckerFeedback =
       new Promise(resolve => {
         ipcMain.on('spec-spell-check', (e, words, callback) => {
-          if (words.length === 2) {
-            // The promise is resolved only after this event is received twice
-            // Array contains only 1 word first time and 2 the next time
+          if (words.length === 5) {
+            // The API calls the provider after every completed word.
+            // The promise is resolved only after this event is received with all words.
             resolve([words, callback])
           }
         })
       })
-    const inputText = 'spleling test '
+    const inputText = `spleling test you're `
     for (const keyCode of inputText) {
       w.webContents.sendInputEvent({ type: 'char', keyCode })
     }
     const [words, callback] = await spellCheckerFeedback
-    expect(words).to.deep.equal(['spleling', 'test'])
+    expect(words.sort()).to.deep.equal(['spleling', 'test', `you're`, 'you', 're'].sort())
     expect(callback).to.be.true()
   })