Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upDeprecate electron spellchecker by chromium's #244
Conversation
| if (spellcheck) { | ||
| if (spellcheck->GetCustomDictionary() | ||
| ->HasWord(base::UTF16ToUTF8(new_params.selection_text))) { | ||
| new_params.misspelled_word = |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
darkdh
Jul 11, 2017
Author
Member
It is used for https://github.com/brave/browser-laptop/pull/9951/files#diff-b9f459fd47114771333b78b87fac7daaR557 which provides user an option to remove added word to custom dictionary.
because if a word is in custom dictionary, we can't tell if it is correctly spelled word or in custom dictionary from ContextMenuParams. misspelled_word and dictionary_suggestions will be empty
This comment has been minimized.
This comment has been minimized.
darkdh
Jul 11, 2017
Author
Member
per discussion on slack, we should use ContextMenuParams.properties
| @@ -971,6 +989,50 @@ void WebContents::AuthorizePlugin(mate::Arguments* args) { | |||
| web_contents(), true, resource_id); | |||
| } | |||
|
|
|||
| void WebContents::AddWord(mate::Arguments* args) { | |||
This comment has been minimized.
This comment has been minimized.
bridiver
Jul 11, 2017
Collaborator
the spellcheck service is associated with the browser context, not the webcontents so this should probably be an api attached to session like autofill
This comment has been minimized.
This comment has been minimized.
darkdh
Jul 11, 2017
Author
Member
will do. and this will helps for migrating old words in legacy custom dictionary.
| if (SpellCheckProvider* provider = | ||
| SpellCheckProvider::Get(render_view->GetMainRenderFrame())) | ||
| provider->EnableSpellcheck(spellcheck_->IsSpellcheckEnabled()); | ||
| new SpellCheckPanel(render_view); |
This comment has been minimized.
This comment has been minimized.
bridiver
Jul 11, 2017
Collaborator
shouldn't this be wrapped in #if BUILDFLAG(HAS_SPELLCHECK_PANEL) ?
This comment has been minimized.
This comment has been minimized.
darkdh
Jul 11, 2017
Author
Member
yes, we should. because I was working based on cr59 and this is added in cr60.
When I pushed the commit, I just rebase the muon for our cr60 commits
| Emit("context-menu", std::make_pair(params, web_contents())); | ||
| // For forgetting custom dictionary option | ||
| content::ContextMenuParams new_params(params); | ||
| if (new_params.misspelled_word.empty()) { |
This comment has been minimized.
This comment has been minimized.
|
@bridiver feedback addressed! |
|
++ with a few small changes |
| if (spellcheck->GetCustomDictionary()->HasWord(selection_text)) { | ||
| new_params.properties.insert( | ||
| std::pair<std::string, std::string> | ||
| (std::string("learnedSpelling"), selection_text)); |
This comment has been minimized.
This comment has been minimized.
| @@ -71,36 +67,9 @@ WebFrameBindings::~WebFrameBindings() { | |||
| } | |||
|
|
|||
| void WebFrameBindings::Invalidate() { | |||
This comment has been minimized.
This comment has been minimized.
| Emit("context-menu", std::make_pair(params, web_contents())); | ||
| // For forgetting custom dictionary option | ||
| content::ContextMenuParams new_params(params); | ||
| if (new_params.misspelled_word.empty() && |
This comment has been minimized.
This comment has been minimized.
bridiver
Jul 11, 2017
Collaborator
actually we should also check is_editable and spellcheck_enabled because spellcheck only applies to editable fields. Just want to narrow this check to run as infrequently as possible
|
lgtm. Just want to verify that this is targeted to browser-laptop 0.18? If not please update the milestone to 4.4.0 |
|
my bad.it should be in 4.4.0 |
Auditors: @bridiver, @bbondy, @bsclifton
Deprecate electron spellchecker by chromium's
darkdh commentedJul 11, 2017
Auditors: @bridiver, @bbondy, @bsclifton