From a9ca1520692085264190913412fd43a63c052903 Mon Sep 17 00:00:00 2001 From: Nitish Sakhawalkar Date: Thu, 18 Oct 2018 09:11:53 -0700 Subject: [PATCH] feat: Spellchecker Async Implementation (#14032) * feat:Spellchecker Async Implementation * Adhere to chromium style * Updating dependency to use gh branch * Update docs and electron-typescript-definitions module * Fix lint * Update electron typescript definitions version * Update spec * Address review --- .../api/atom_api_spell_check_client.cc | 152 ++++++++++-------- .../api/atom_api_spell_check_client.h | 33 ++-- atom/renderer/api/atom_api_web_frame.cc | 5 +- atom/renderer/api/atom_api_web_frame.h | 1 - docs/api/web-frame.md | 26 +-- package-lock.json | 6 +- package.json | 2 +- spec/api-web-frame-spec.js | 20 ++- spec/fixtures/pages/webframe-spell-check.html | 7 +- 9 files changed, 141 insertions(+), 111 deletions(-) diff --git a/atom/renderer/api/atom_api_spell_check_client.cc b/atom/renderer/api/atom_api_spell_check_client.cc index 059f7f7017b44..848f0f9282a06 100644 --- a/atom/renderer/api/atom_api_spell_check_client.cc +++ b/atom/renderer/api/atom_api_spell_check_client.cc @@ -4,7 +4,7 @@ #include "atom/renderer/api/atom_api_spell_check_client.h" -#include +#include #include #include "atom/common/native_mate_converters/string16_converter.h" @@ -13,6 +13,7 @@ #include "chrome/renderer/spellchecker/spellcheck_worditerator.h" #include "native_mate/converter.h" #include "native_mate/dictionary.h" +#include "native_mate/function_template.h" #include "third_party/blink/public/web/web_text_checking_completion.h" #include "third_party/blink/public/web/web_text_checking_result.h" #include "third_party/icu/source/common/unicode/uscript.h" @@ -40,6 +41,10 @@ bool HasWordCharacters(const base::string16& text, int index) { class SpellCheckClient::SpellcheckRequest { public: + // Map of individual words to list of occurrences in text + using WordMap = + std::map>; + SpellcheckRequest(const base::string16& text, blink::WebTextCheckingCompletion* completion) : text_(text), completion_(completion) { @@ -47,12 +52,13 @@ class SpellCheckClient::SpellcheckRequest { } ~SpellcheckRequest() {} - base::string16 text() { return text_; } + const base::string16& text() const { return text_; } blink::WebTextCheckingCompletion* completion() { return completion_; } + WordMap& wordmap() { return word_map_; } private: base::string16 text_; // Text to be checked in this task. - + WordMap word_map_; // WordMap to hold distinct words in text // The interface to send the misspelled ranges to WebKit. blink::WebTextCheckingCompletion* completion_; @@ -60,10 +66,10 @@ class SpellCheckClient::SpellcheckRequest { }; SpellCheckClient::SpellCheckClient(const std::string& language, - bool auto_spell_correct_turned_on, v8::Isolate* isolate, v8::Local provider) - : isolate_(isolate), + : pending_request_param_(nullptr), + isolate_(isolate), context_(isolate, isolate->GetCurrentContext()), provider_(isolate, provider) { DCHECK(!context_.IsEmpty()); @@ -79,19 +85,6 @@ SpellCheckClient::~SpellCheckClient() { context_.Reset(); } -void SpellCheckClient::CheckSpelling( - const blink::WebString& text, - int& misspelling_start, - int& misspelling_len, - blink::WebVector* optional_suggestions) { - std::vector results; - SpellCheckText(text.Utf16(), true, &results); - if (results.size() == 1) { - misspelling_start = results[0].location; - misspelling_len = results[0].length; - } -} - void SpellCheckClient::RequestCheckingOfText( const blink::WebString& textToCheck, blink::WebTextCheckingCompletion* completionCallback) { @@ -103,7 +96,7 @@ void SpellCheckClient::RequestCheckingOfText( } // Clean up the previous request before starting a new request. - if (pending_request_param_.get()) { + if (pending_request_param_) { pending_request_param_->completion()->DidCancelCheckingText(); } @@ -111,8 +104,7 @@ void SpellCheckClient::RequestCheckingOfText( base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, - base::BindOnce(&SpellCheckClient::PerformSpellCheck, AsWeakPtr(), - base::Owned(pending_request_param_.release()))); + base::BindOnce(&SpellCheckClient::SpellCheckText, AsWeakPtr())); } bool SpellCheckClient::IsSpellCheckingEnabled() const { @@ -128,12 +120,13 @@ bool SpellCheckClient::IsShowingSpellingUI() { void SpellCheckClient::UpdateSpellingUIWithMisspelledWord( const blink::WebString& word) {} -void SpellCheckClient::SpellCheckText( - const base::string16& text, - bool stop_at_first_result, - std::vector* results) { - if (text.empty() || spell_check_.IsEmpty()) +void SpellCheckClient::SpellCheckText() { + const auto& text = pending_request_param_->text(); + if (text.empty() || spell_check_.IsEmpty()) { + pending_request_param_->completion()->DidCancelCheckingText(); + pending_request_param_ = nullptr; return; + } if (!text_iterator_.IsInitialized() && !text_iterator_.Initialize(&character_attributes_, true)) { @@ -153,56 +146,87 @@ void SpellCheckClient::SpellCheckText( SpellCheckScope scope(*this); base::string16 word; - int word_start; - int word_length; - for (auto status = - text_iterator_.GetNextWord(&word, &word_start, &word_length); - status != SpellcheckWordIterator::IS_END_OF_TEXT; - status = text_iterator_.GetNextWord(&word, &word_start, &word_length)) { + std::vector words; + auto& word_map = pending_request_param_->wordmap(); + blink::WebTextCheckingResult result; + for (;;) { // Run until end of text + const auto status = + text_iterator_.GetNextWord(&word, &result.location, &result.length); + if (status == SpellcheckWordIterator::IS_END_OF_TEXT) + break; if (status == SpellcheckWordIterator::IS_SKIPPABLE) continue; - // Found a word (or a contraction) that the spellchecker can check the - // spelling of. - if (SpellCheckWord(scope, word)) - continue; - // 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. - if (IsValidContraction(scope, word)) - continue; + 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); + } + } + } - blink::WebTextCheckingResult result; - result.location = word_start; - result.length = word_length; - results->push_back(result); + // Send out all the words data to the spellchecker to check + SpellCheckWords(scope, words); +} - if (stop_at_first_result) - return; +void SpellCheckClient::OnSpellCheckDone( + const std::vector& misspelled_words) { + std::vector results; + auto* const completion_handler = pending_request_param_->completion(); + + 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(); + } } + completion_handler->DidFinishCheckingText(results); + pending_request_param_ = nullptr; } -bool SpellCheckClient::SpellCheckWord( +void SpellCheckClient::SpellCheckWords( const SpellCheckScope& scope, - const base::string16& word_to_check) const { + const std::vector& words) { DCHECK(!scope.spell_check_.IsEmpty()); - v8::Local word = mate::ConvertToV8(isolate_, word_to_check); - v8::Local result = - scope.spell_check_->Call(scope.provider_, 1, &word); + v8::Local templ = mate::CreateFunctionTemplate( + isolate_, base::Bind(&SpellCheckClient::OnSpellCheckDone, AsWeakPtr())); - if (!result.IsEmpty() && result->IsBoolean()) - return result->BooleanValue(); - else - return true; + v8::Local args[] = {mate::ConvertToV8(isolate_, words), + templ->GetFunction()}; + // Call javascript with the words and the callback function + scope.spell_check_->Call(scope.provider_, 2, args); } -// Returns whether or not the given string is a valid contraction. +// Returns whether or not the given string is a contraction. // This function is a fall-back when the SpellcheckWordIterator class // returns a concatenated word which is not in the selected dictionary // (e.g. "in'n'out") but each word is valid. -bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope, - const base::string16& contraction) { +// Output variable contraction_words will contain individual +// words in the contraction. +bool SpellCheckClient::IsContraction( + const SpellCheckScope& scope, + const base::string16& contraction, + std::vector* contraction_words) { DCHECK(contraction_iterator_.IsInitialized()); contraction_iterator_.SetText(contraction.c_str(), contraction.length()); @@ -210,7 +234,6 @@ bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope, base::string16 word; int word_start; int word_length; - for (auto status = contraction_iterator_.GetNextWord(&word, &word_start, &word_length); status != SpellcheckWordIterator::IS_END_OF_TEXT; @@ -219,18 +242,9 @@ bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope, if (status == SpellcheckWordIterator::IS_SKIPPABLE) continue; - if (!SpellCheckWord(scope, word)) - return false; + contraction_words->push_back(word); } - return true; -} - -void SpellCheckClient::PerformSpellCheck(SpellcheckRequest* param) { - DCHECK(param); - - std::vector results; - SpellCheckText(param->text(), false, &results); - param->completion()->DidFinishCheckingText(results); + return contraction_words->size() > 1; } SpellCheckClient::SpellCheckScope::SpellCheckScope( diff --git a/atom/renderer/api/atom_api_spell_check_client.h b/atom/renderer/api/atom_api_spell_check_client.h index 79dc8ffe3e704..aa2b4214ad125 100644 --- a/atom/renderer/api/atom_api_spell_check_client.h +++ b/atom/renderer/api/atom_api_spell_check_client.h @@ -31,7 +31,6 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient, public base::SupportsWeakPtr { public: SpellCheckClient(const std::string& language, - bool auto_spell_correct_turned_on, v8::Isolate* isolate, v8::Local provider); ~SpellCheckClient() override; @@ -39,11 +38,6 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient, private: class SpellcheckRequest; // blink::WebTextCheckClient: - void CheckSpelling( - const blink::WebString& text, - int& misspelledOffset, - int& misspelledLength, - blink::WebVector* optionalSuggestions) override; void RequestCheckingOfText( const blink::WebString& textToCheck, blink::WebTextCheckingCompletion* completionCallback) override; @@ -65,22 +59,27 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient, ~SpellCheckScope(); }; - // Check the spelling of text. - void SpellCheckText(const base::string16& text, - bool stop_at_first_result, - std::vector* results); + // Run through the word iterator and send out requests + // to the JS API for checking spellings of words in the current + // request. + void SpellCheckText(); // Call JavaScript to check spelling a word. - bool SpellCheckWord(const SpellCheckScope& scope, - const base::string16& word_to_check) const; + // The javascript function will callback OnSpellCheckDone + // with the results of all the misspelled words. + void SpellCheckWords(const SpellCheckScope& scope, + const std::vector& words); // Returns whether or not the given word is a contraction of valid words // (e.g. "word:word"). - bool IsValidContraction(const SpellCheckScope& scope, - const base::string16& word); - - // Performs spell checking from the request queue. - void PerformSpellCheck(SpellcheckRequest* param); + // Output variable contraction_words will contain individual + // words in the contraction. + bool IsContraction(const SpellCheckScope& scope, + const base::string16& word, + std::vector* contraction_words); + + // Callback for the JS API which returns the list of misspelled words. + void OnSpellCheckDone(const std::vector& misspelled_words); // Represents character attributes used for filtering out characters which // are not supported by this SpellCheck object. diff --git a/atom/renderer/api/atom_api_web_frame.cc b/atom/renderer/api/atom_api_web_frame.cc index 0e96bc135053a..873575a6030ae 100644 --- a/atom/renderer/api/atom_api_web_frame.cc +++ b/atom/renderer/api/atom_api_web_frame.cc @@ -215,15 +215,14 @@ int WebFrame::GetWebFrameId(v8::Local content_window) { void WebFrame::SetSpellCheckProvider(mate::Arguments* args, const std::string& language, - bool auto_spell_correct_turned_on, v8::Local provider) { if (!provider->Has(mate::StringToV8(args->isolate(), "spellCheck"))) { args->ThrowError("\"spellCheck\" has to be defined"); return; } - auto client = std::make_unique( - language, auto_spell_correct_turned_on, args->isolate(), provider); + auto client = + std::make_unique(language, args->isolate(), provider); // Set spellchecker for all live frames in the same process or // in the sandbox mode for all live sub frames to this WebFrame. FrameSpellChecker spell_checker( diff --git a/atom/renderer/api/atom_api_web_frame.h b/atom/renderer/api/atom_api_web_frame.h index 9fbdb165e6972..9b368670dbf65 100644 --- a/atom/renderer/api/atom_api_web_frame.h +++ b/atom/renderer/api/atom_api_web_frame.h @@ -58,7 +58,6 @@ class WebFrame : public mate::Wrappable { // Set the provider that will be used by SpellCheckClient for spell check. void SetSpellCheckProvider(mate::Arguments* args, const std::string& language, - bool auto_spell_correct_turned_on, v8::Local provider); void RegisterURLSchemeAsBypassingCSP(const std::string& scheme); diff --git a/docs/api/web-frame.md b/docs/api/web-frame.md index ee3d08c783d5e..43612ca8a44f8 100644 --- a/docs/api/web-frame.md +++ b/docs/api/web-frame.md @@ -57,26 +57,34 @@ Sets the maximum and minimum pinch-to-zoom level. Sets the maximum and minimum layout-based (i.e. non-visual) zoom level. -### `webFrame.setSpellCheckProvider(language, autoCorrectWord, provider)` +### `webFrame.setSpellCheckProvider(language, provider)` * `language` String -* `autoCorrectWord` Boolean * `provider` Object - * `spellCheck` Function - Returns `Boolean`. - * `text` String + * `spellCheck` Function. + * `words` String[] + * `callback` Function + * `misspeltWords` String[] Sets a provider for spell checking in input fields and text areas. -The `provider` must be an object that has a `spellCheck` method that returns -whether the word passed is correctly spelled. +The `provider` must be an object that has a `spellCheck` method that accepts +an array of individual words for spellchecking. +The `spellCheck` function runs asynchronously and calls the `callback` function +with an array of misspelt words when complete. An example of using [node-spellchecker][spellchecker] as provider: ```javascript const { webFrame } = require('electron') -webFrame.setSpellCheckProvider('en-US', true, { - spellCheck (text) { - return !(require('spellchecker').isMisspelled(text)) +const spellChecker = require('spellchecker') +webFrame.setSpellCheckProvider('en-US', { + spellCheck (words, callback) { + setTimeout(() => { + const spellchecker = require('spellchecker') + const misspelled = words.filter(x => spellchecker.isMisspelled(x)) + callback(misspelled) + }, 0) } }) ``` diff --git a/package-lock.json b/package-lock.json index 90dc1ce316025..5a9707ad22c56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2860,9 +2860,9 @@ } }, "electron-typescript-definitions": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/electron-typescript-definitions/-/electron-typescript-definitions-2.1.1.tgz", - "integrity": "sha512-vrEhi3hhPzUEDLwPGOqScYBLefNKH5r9odp3dy/lqE0nhAmUHBkrwnU5jVga3A2pJW22wzCCB1kwkEoPV7Rq4w==", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/electron-typescript-definitions/-/electron-typescript-definitions-3.0.0.tgz", + "integrity": "sha512-PQEdLGY9i5IWxQKWCllCun3Lh07YDMeTt/YRbsS+kaCC13vvxoy/LprwkqRUWbzAH704MHtDU9pKrEK2MoqP2w==", "dev": true, "requires": { "@types/node": "^7.0.18", diff --git a/package.json b/package.json index 65b0768bda7f5..d462c142b2c3a 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "dugite": "^1.45.0", "electabul": "~0.0.4", "electron-docs-linter": "^2.4.0", - "electron-typescript-definitions": "^2.1.1", + "electron-typescript-definitions": "^3.0.0", "eslint": "^5.6.0", "eslint-config-standard": "^12.0.0", "eslint-plugin-mocha": "^5.2.0", diff --git a/spec/api-web-frame-spec.js b/spec/api-web-frame-spec.js index 171d828286db2..3f5529c3af392 100644 --- a/spec/api-web-frame-spec.js +++ b/spec/api-web-frame-spec.js @@ -152,12 +152,22 @@ describe('webFrame module', function () { w.focus() await w.webContents.executeJavaScript('document.querySelector("input").focus()', true) - const spellCheckerFeedback = emittedOnce(ipcMain, 'spec-spell-check') - const misspelledWord = 'spleling' - for (const keyCode of [...misspelledWord, ' ']) { + const spellCheckerFeedback = + new Promise((resolve, reject) => { + 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 + resolve([words, callback]) + } + }) + }) + const inputText = 'spleling test ' + for (const keyCode of inputText) { w.webContents.sendInputEvent({ type: 'char', keyCode }) } - const [, text] = await spellCheckerFeedback - expect(text).to.equal(misspelledWord) + const [words, callback] = await spellCheckerFeedback + expect(words).to.deep.equal(['spleling', 'test']) + expect(callback).to.be.true() }) }) diff --git a/spec/fixtures/pages/webframe-spell-check.html b/spec/fixtures/pages/webframe-spell-check.html index db83964e55fdb..8b162201a4d30 100644 --- a/spec/fixtures/pages/webframe-spell-check.html +++ b/spec/fixtures/pages/webframe-spell-check.html @@ -2,9 +2,10 @@