Windows: Spell checker not working, add support for WinUseBrowserSpellChecker

[magreenblatt]

Original report by Salvador Diaz Fau (Bitbucket: salvadordf, GitHub: salvadordf).

---

What steps will reproduce the problem?

• Run cefclient.exe
• Load https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_textarea
• Type some word with spelling errors in the TEXTAREA element like “yelow” and the word won’t be marked with a red squiggly line.

‌

What is the expected output? What do you see instead?

Words with incorrect spelling should be marked with a red squiggly line underneath but it doesn’t add the red line to any word.

‌

What version of the product are you using? On what operating system?

CEF 87.1.11 on Windows 10 64 bits
https://cef-builds.spotifycdn.com/cef_binary_87.1.11%2Bg8bb7705%2Bchromium-87.0.4280.66_windows64_client.tar.bz2

‌

Does the problem reproduce with the cefclient or cefsimple sample application at the same version? How about with a newer or older version?

CEF 86.0.24 worked correctly and the incorrectly spelled words had a red line in cefclient.

‌

Does the problem reproduce with Google Chrome at the same version? How about with a newer or older version?

I couldn’t test Chrome.

‌

Additional notes :

• Using the “--lang” and “--cache-path” command line switches didn’t solve this issue.
• The %LOCALAPPDATA%\CEF\User Data\Dictionaries directory is created but cefclient doesn’t download the default english dictionary. However, the older cefclient (M86) downloads the english dictionary correctly.

‌

‌

[magreenblatt]

Original comment by Salvador Diaz Fau (Bitbucket: salvadordf, GitHub: salvadordf).

---

Workaround : Disabling the WinUseBrowserSpellChecker feature seems to fix this issue.

cefclient --disable-features=WinUseBrowserSpellChecker

Chromium enables the Windows native spell checker by default :
https://chromium.googlesource.com/chromium/src/+/2a7dd7be5e60c7ccde8d16651f737f9d1e220d63

‌

[magreenblatt]

• changed component from "Unclassified" to "Framework"
• changed title from "Spell checker not working" to "Windows: Spell checker not working, add support for WinUseBrowserSpellChecker"
• changed kind from "bug" to "enhancement"

[magreenblatt]

alloy-win: Disable the WinUseBrowserSpellChecker feature (see issue #3055)

Workaround until support for the Windows 10+ spellcheck service is added for
the Alloy runtime.

→ <<cset eabdf3a2ca32 (bb)>>

[magreenblatt]

alloy-win: Disable the WinUseBrowserSpellChecker feature (see issue #3055)

Workaround until support for the Windows 10+ spellcheck service is added for
the Alloy runtime.

→ <<cset 192276af566c (bb)>>

[magreenblatt]

alloy-win: Disable the WinUseBrowserSpellChecker feature (see issue #3055)

Workaround until support for the Windows 10+ spellcheck service is added for
the Alloy runtime.

→ <<cset b356923f5c73 (bb)>>

[bjdupuis]

alloy-win: Disable the WinUseBrowserSpellChecker feature (see issue #3055)

Workaround until support for the Windows 10+ spellcheck service is added for the Alloy runtime.

→ <<cset b356923f5c73 (bb)>>

I notice that this flag got removed with this commit. Does that mean the spellcheck service is supported in that version and this flag is no longer required? I ask because we've had the WinUseBrowserSpellChecker feature manually disabled since this issue was opened and it's never been closed so we haven't removed it.

[magreenblatt]

This should work with Chrome bootstrap. See #3685.

[magreenblatt]

The WinUseBrowserSpellChecker feature flag no longer exists in Chromium, so it cannot be disabled.

Something may be required to support this with Alloy style. See https://www.magpcss.org/ceforum/viewtopic.php?f=6&t=18269&start=10

[bjdupuis]

Is there any potential guidance on where to possibly start with triaging and fixing this in Alloy style? Our users are perhaps unsurprisingly disappointed with losing spell check and I feel powerless to impact it. Telling them "just stick with the pre-chrome-runtime version" only works until there's a serious enough security fix in Chromium to get out.

[magreenblatt]

From the forum thead:

Now with the chrome runtime I get the red underline but "No spelling suggestions" in the context menu. Looking at the params->GetDictionarySuggestions() there are no suggestions to put into the context menu manually.

This suggests that spellchecking is at least partially working with Alloy style. GetDictionarySuggestions() is just a thin wrapper around ContextMenuParams.dictionary_suggestions. The suggestions should come from the renderer process here.

[bjdupuis]

Is the Alloy bootstrap code available to reference to see what steps it took to get the suggestions in there that's absent in the Chrome bootstrap?

[magreenblatt]

As mentioned above, spellchecker with Alloy only worked when running with --disable-features=WinUseBrowserSpellChecker. The ability to disable this feature has been removed in newer Chromium versions, which is likely why it doesn't work now. Referring to old Alloy code will not help you with this.

[magreenblatt]

Native spellchecker was implemented in https://issues.chromium.org/issues/40407926. Referring to those commits may assist you.

[amaitland]

Looks like the ability to disable the specific feature was removed in https://chromium.googlesource.com/chromium/src/+/3fed1eefc32dd0b0aedd428af13c4a199dc863f6

Looks like there is still a way to disable in code, so in theory you could build a custom version of CEF.

[amaitland]

There is a preference to disable the spell check service, might be worth trying https://source.chromium.org/chromium/chromium/src/+/main:components/spellcheck/browser/pref_names.h;drc=e3f856ae03dd1c131b435a16d71f36de3f0f44a3;l=32 to see if works as a temp workaround (I haven't tested yet)

[mol]

Thanks @amaitland. I've been researching this issue heavily the last week, and so far it has led me to create #3860

Disabling off screen rendering in CefSharp's minimal example didn't immediately solve the issue though, but we have a lot of users affected by this, so I'll take a look at what you found and continue to look for a solution.

If you have any other ideas where else I might check, with the above in mind - let me know :)

[amaitland]

heavily the last week, and so far it has led me to create #3860

@mol I suspect your issue is a duplicate of this one. I suspect the cases it works are where the spell check falls back to hunspell as windows doesn't have the relevant language pack (last paragraph of https://issues.chromium.org/issues/40247740)

Disabling off screen rendering in CefSharp's minimal example didn't immediately solve the issue though,

Currently spell check only works with chrome styled windows. In the context of CefSharp using the hwndhost example and switching to chrome style should work. cefsharp/CefSharp#4835 (comment)

If you have any other ideas where else I might check, with the above in mind - let me know :)

Will do.

[mol]

I suspect the cases it works are where the spell check falls back to hunspell as windows doesn't have the relevant language pack (last paragraph of https://issues.chromium.org/issues/40247740)

That could have been the issue, but my Windows language is en-US (one of the ones that doesn't work for me in CEF), and spell checking works in Windows' settings:

Currently spell check only works with chrome styled windows. In the context of CefSharp using the hwndhost example and switching to chrome style should work. cefsharp/CefSharp#4835 (comment)

Thanks. I was considering playing around more with the hwndhost version, but ran into issues last time I briefly tried it. Our code that's using CefSharp is pretty ancient, so needs a bit of work :)

[amaitland]

There is a preference to disable the spell check service, might be worth trying https://source.chromium.org/chromium/chromium/src/+/main:components/spellcheck/browser/pref_names.h;drc=e3f856ae03dd1c131b435a16d71f36de3f0f44a3;l=32 to see if works as a temp workaround (I haven't tested yet)

Unfortunately this didn't appear to do anything meaningful.

[mol]

There is a preference to disable the spell check service, might be worth trying https://source.chromium.org/chromium/chromium/src/+/main:components/spellcheck/browser/pref_names.h;drc=e3f856ae03dd1c131b435a16d71f36de3f0f44a3;l=32 to see if works as a temp workaround (I haven't tested yet)

Unfortunately this didn't appear to do anything meaningful.

Thanks for trying 👍

I also tried setting use_browser_spellchecker=false in GN_DEFINES, when building a custom CEF version (to disable the platform spell checker), but it seems it's overruled here. Then I tried setting it to false in there explicitly, but it caused a build error, where the build flag was causing some necessary code not to be included. I'm thinking the flag is likely so old it hasn't been maintained over the years. I'll see if I can find a way around it.

I might also try and see if I can get to actually run and debug CEF with Chromium. That would make all this so much easier - being able to step through and see what's happening.

[mol]

Trying to get the Chromium USE_BROWSER_SPELLCHECKER build flag to work led me down a rabbit hole of changes.

Instead, changing UseBrowserSpellChecker to return false did the same thing of disabling the Windows spell checker, and now spell checking with suggestions works properly in Alloy for all languages, installed or otherwise.

Obviously it would be nicer to add support for the Windows spell checker for Alloy style, but this works for me - as I anyway do a custom CEF build.

[mol]

If you're on a version of Chromium prior to two weeks ago, I also found a much easier way to get it working - without requiring a custom build.

Just use the command line argument --disable-features=WinRetrieveSuggestionsOnlyOnDemand.

But this wouldn't work moving forward, as the feature flag is now removed.

It seems like to support the Windows spell checker, Alloy CEF would need to listen to the async changes to the suggestions, I think from here, and then re-create the context menu here (or replace the existing suggestions inside). I'm not very familiar with CEF, or C++ anymore for that matter, but I'll have a look and see if I can work it out.

[amaitland]

Looks like Electron were disabling WinRetrieveSuggestionsOnlyOnDemand to get spell check working, as that no longer works they have made changes to support async suggestions.

electron/electron@47f08d8

I've only had a quick look over the code, no sure if the same solution will apply yet.

[mbragg12]

Scratch my other idea of implementing the add and update handlers. The electron solution was way easier. I should have a patch shortly.

[mbragg12]

Here is the patch for 131. It will be a bit for me to switch back to master and make a PR.

@magreenblatt let me know if you have any issue with this approach.

diff --git a/libcef/browser/menu_manager.cc b/libcef/browser/menu_manager.cc
index 6bdf880e6..3d420d3d6 100644
--- a/libcef/browser/menu_manager.cc
+++ b/libcef/browser/menu_manager.cc
@@ -14,7 +14,12 @@
 #include "cef/libcef/browser/menu_runner.h"
 #include "cef/libcef/browser/thread_util.h"
 #include "cef/libcef/common/app_manager.h"
+#include "chrome/browser/spellchecker/spellcheck_factory.h"
+#include "chrome/browser/spellchecker/spellcheck_service.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/spellcheck/browser/spellcheck_platform.h"
+#include "components/spellcheck/common/spellcheck_common.h"
+#include "components/spellcheck/common/spellcheck_features.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_view.h"
@@ -120,8 +125,8 @@ bool CefMenuManager::IsShowingContextMenu() {
   return web_contents()->IsShowingContextMenu();
 }
 
-bool CefMenuManager::CreateContextMenu(
-    const content::ContextMenuParams& params) {
+bool CefMenuManager::CreateContextMenu(const content::ContextMenuParams& params,
+                                       bool allow_spellcheck) {
   // The renderer may send the "show context menu" message multiple times, one
   // for each right click mouse event it receives. Normally, this doesn't happen
   // because mouse events are not forwarded once the context menu is showing.
@@ -134,6 +139,25 @@ bool CefMenuManager::CreateContextMenu(
   }
 
   params_ = params;
+
+#if BUILDFLAG(IS_WIN)
+  // System spellcheck suggestions are generated asyncronously.  So fetch the
+  // suggestions first.
+  if (allow_spellcheck && !params_.misspelled_word.empty() &&
+      params_.dictionary_suggestions.empty()) {
+    SpellcheckService* spellcheck_service =
+        SpellcheckServiceFactory::GetForContext(
+            browser_->web_contents()->GetBrowserContext());
+    if (spellcheck_service) {
+      spellcheck_platform::GetPerLanguageSuggestions(
+          spellcheck_service->platform_spell_checker(), params_.misspelled_word,
+          base::BindOnce(&CefMenuManager::OnGetPlatformSuggestionsComplete,
+                         weak_ptr_factory_.GetWeakPtr()));
+    }
+    return true;
+  }
+#endif
+
   model_->Clear();
 
   // Create the default menu model.
@@ -202,6 +226,19 @@ bool CefMenuManager::CreateContextMenu(
   return true;
 }
 
+void CefMenuManager::OnGetPlatformSuggestionsComplete(
+    const spellcheck::PerLanguageSuggestions&
+        platform_per_language_suggestions) {
+  std::vector<std::u16string> combined_suggestions;
+  spellcheck::FillSuggestions(platform_per_language_suggestions,
+                              &combined_suggestions);
+
+  params_.dictionary_suggestions = combined_suggestions;
+
+  // Now that we have spelling suggestions, call CreateContextMenu again.
+  CreateContextMenu(params_, false);
+}
+
 void CefMenuManager::CancelContextMenu() {
   if (IsShowingContextMenu()) {
     if (custom_menu_callback_) {

warning: in the working copy of 'libcef/browser/menu_manager.cc', LF will be replaced by CRLF the next time Git touches it
diff --git a/libcef/browser/menu_manager.h b/libcef/browser/menu_manager.h
index e49234147..f753e9f0f 100644
--- a/libcef/browser/menu_manager.h
+++ b/libcef/browser/menu_manager.h
@@ -10,6 +10,7 @@
 #include "base/memory/weak_ptr.h"
 #include "cef/libcef/browser/menu_model_impl.h"
 #include "cef/libcef/browser/menu_runner.h"
+#include "components/spellcheck/common/spellcheck_common.h"
 #include "content/public/browser/context_menu_params.h"
 #include "content/public/browser/web_contents_observer.h"
 
@@ -39,9 +40,14 @@ class CefMenuManager : public CefMenuModelImpl::Delegate,
   bool IsShowingContextMenu();
 
   // Create the context menu.
-  bool CreateContextMenu(const content::ContextMenuParams& params);
+  bool CreateContextMenu(const content::ContextMenuParams& params,
+                         bool allow_spellcheck = true);
   void CancelContextMenu();
 
+  void OnGetPlatformSuggestionsComplete(
+      const spellcheck::PerLanguageSuggestions&
+          platform_per_language_suggestions);
+
  private:
   // CefMenuModelImpl::Delegate methods.
   void ExecuteCommand(CefRefPtr<CefMenuModelImpl> source,

[mbragg12]

PR
https://bitbucket.org/chromiumembedded/cef/pull-requests/859