fix: contractions handling in spellchecker (#18548)

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 <vector>
 
 #include "atom/common/native_mate_converters/string16_converter.h"
@@ -37,14 +39,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,
                     blink::WebTextCheckingCompletion* completion)
       : text_(text), completion_(completion) {
@@ -54,11 +58,11 @@ class SpellCheckClient::SpellcheckRequest {
 
   const base::string16& text() const { return text_; }
   blink::WebTextCheckingCompletion* completion() { return completion_; }
-  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.
   blink::WebTextCheckingCompletion* completion_;
 
@@ -146,31 +150,27 @@ void SpellCheckClient::SpellCheckText() {
 
   SpellCheckScope scope(*this);
   base::string16 word;
-  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, &result.location, &result.length);
+    const auto status = text_iterator_.GetNextWord(
+        &word, &word_entry.result.location, &word_entry.result.length);
     if (status == SpellcheckWordIterator::IS_END_OF_TEXT)
       break;
     if (status == SpellcheckWordIterator::IS_SKIPPABLE)
       continue;
 
+    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);
       }
     }
   }
@@ -183,29 +183,34 @@ void SpellCheckClient::OnSpellCheckDone(
     const std::vector<base::string16>& misspelled_words) {
   std::vector<blink::WebTextCheckingResult> results;
   auto* const completion_handler = pending_request_param_->completion();
+  std::unordered_set<base::string16> misspelled(misspelled_words.begin(),
+                                                misspelled_words.end());
+  auto& word_list = pending_request_param_->wordlist();
 
-  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();
+  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);
     }
   }
   completion_handler->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

@@ -41,19 +41,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()
   })