electron / electron

:electron: Build cross-platform desktop apps with JavaScript, HTML, and CSS
https://electronjs.org
MIT License
114.43k stars 15.44k forks source link

[Feature Request]: Support Windows spell check for non-Hunspell languages #39906

Open jeremyspiegel opened 1 year ago

jeremyspiegel commented 1 year ago

Preflight Checklist

Problem Description

The session.setSpellCheckerLanguages API currently only supports the hard-coded list of languages supported by Hunspell in kSupportedSpellCheckerLanguages in components/spellcheck/common/spellcheck_common.cc. This means that Electron doesn't provide spellcheck support for languages without Hunspell support (like Finnish), even though these languages are supported by the Windows API Electron already uses for Hunspell-compatible languages.

Chromium supports these, however, with support added in https://chromium-review.googlesource.com/c/chromium/src/+/2155167.

Proposed Solution

In the implementation for session.setSpellCheckerLanguages (Session::SetSpellCheckerLanguages), if I remove the spellcheck::GetCorrespondingSpellCheckLanguage call and just pass the language code through to the spellcheck::prefs::kSpellCheckDictionaries pref as-is, I’m able to use it in a test app by calling session.setSpellCheckerLanguages(['fi']) (for Finnish) and see Finnish spellchecking work in Electron (the API would otherwise throw the "Invalid language code" error).

Rather than remove the spellcheck::GetCorrespondingSpellCheckLanguage check, I suppose we'd need to supplement it with a check that the language passes spellcheck_platform::PlatformSupportsLanguage or is in the result of spellcheck_platform::RetrieveSpellcheckLanguages, though that would require the API change to async.

I think fixing this would also require either changing session.availableSpellCheckerLanguages to also return the supported languages for which the Windows language pack is installed (via spellcheck_platform::RetrieveSpellcheckLanguages), or adding a new API for that (since it seems it would need to be retrieved async).

Maybe the list of available languages can be retrieved when a session is created?

Alternatives Considered

n/a

Additional Information

No response

jeremyspiegel commented 1 year ago

FYI @MarshallOfSound. Sorry this is what I was trying to accomplish when I filed https://github.com/electron/electron/issues/31703 but I didn't understand how the hybrid spell checker worked at the time so I wasn't able to describe the issue as clearly.

I can put together a PR if it would be welcome and if there was a preference on how to handle checking if a language is available and/or modifying session.availableSpellCheckerLanguages.

codebytere commented 1 year ago

@jeremyspiegel that PR proposal sounds good - I'm also happy to review it when you do.

jeremyspiegel commented 1 year ago

@codebytere thank you! Though I'm sorry to say that priorities have changed since I opened this feature request, so I won't be able to work on that PR anytime soon.