From ab70e854f8ab4ee043382f64e427520473d39ce7 Mon Sep 17 00:00:00 2001 From: Maya Wolf Date: Fri, 31 May 2019 06:19:10 +0200 Subject: [PATCH] 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. --- .../api/atom_api_spell_check_client.cc | 87 ++++++++++--------- .../api/atom_api_spell_check_client.h | 3 +- spec/api-web-frame-spec.js | 10 +-- 3 files changed, 53 insertions(+), 47 deletions(-) diff --git a/atom/renderer/api/atom_api_spell_check_client.cc b/atom/renderer/api/atom_api_spell_check_client.cc index e8a5efbed28d3..59689c8de9da7 100644 --- a/atom/renderer/api/atom_api_spell_check_client.cc +++ b/atom/renderer/api/atom_api_spell_check_client.cc @@ -5,6 +5,8 @@ #include "atom/renderer/api/atom_api_spell_check_client.h" #include +#include +#include #include #include @@ -39,14 +41,16 @@ bool HasWordCharacters(const base::string16& text, int index) { return false; } +struct Word { + blink::WebTextCheckingResult result; + base::string16 text; + std::vector contraction_words; +}; + } // namespace class SpellCheckClient::SpellcheckRequest { public: - // Map of individual words to list of occurrences in text - using WordMap = - std::map>; - SpellcheckRequest( const base::string16& text, std::unique_ptr 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& 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_list_; // List of Words found in text // The interface to send the misspelled ranges to WebKit. std::unique_ptr completion_; @@ -150,9 +154,9 @@ void SpellCheckClient::SpellCheckText() { base::string16 word; size_t word_start; size_t word_length; - std::vector words; - auto& word_map = pending_request_param_->wordmap(); - blink::WebTextCheckingResult result; + std::set 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(word_start); - result.length = base::checked_cast(word_length); + word_entry.result.location = base::checked_cast(word_start); + word_entry.result.length = base::checked_cast(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 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& misspelled_words) { std::vector 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 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& words) { +void SpellCheckClient::SpellCheckWords(const SpellCheckScope& scope, + const std::set& words) { DCHECK(!scope.spell_check_.IsEmpty()); v8::Local templ = mate::CreateFunctionTemplate( diff --git a/atom/renderer/api/atom_api_spell_check_client.h b/atom/renderer/api/atom_api_spell_check_client.h index db59201c5b364..66a2b77ffdb2d 100644 --- a/atom/renderer/api/atom_api_spell_check_client.h +++ b/atom/renderer/api/atom_api_spell_check_client.h @@ -6,6 +6,7 @@ #define ATOM_RENDERER_API_ATOM_API_SPELL_CHECK_CLIENT_H_ #include +#include #include #include @@ -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& words); + const std::set& words); // Returns whether or not the given word is a contraction of valid words // (e.g. "word:word"). diff --git a/spec/api-web-frame-spec.js b/spec/api-web-frame-spec.js index 80de523dc20b2..e0a6772b0eade 100644 --- a/spec/api-web-frame-spec.js +++ b/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() })