Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spell Check No Longer Working #135

Closed
cswartzvi opened this issue Feb 20, 2019 · 5 comments · Fixed by #137
Closed

Spell Check No Longer Working #135

cswartzvi opened this issue Feb 20, 2019 · 5 comments · Fixed by #137

Comments

@cswartzvi
Copy link

cswartzvi commented Feb 20, 2019

Misspelled word indicators and context menu word suggestions now longer appear in version 0.90 as tested on Windows 10 and Ubuntu 18.10. This may be directly related to the Messages service itself, because I was unable to remedy the situation by downgrading to version 0.80 (which was working perfectly a few days ago). Interestingly, the app Franz which also provides a wrapper around Messages lost it's ability to spell check as well 😕.

@chrisknepper
Copy link
Owner

Thanks for reporting this @cswartzvi . I took a look at the Franz code and it appears to use the same library (electron-hunspell) and version of Electron (4.0.4). This leads me to believe that Google implemented some sort of killswitch for the way that library is using the WebAssembly feature because they consider compiling or instantiating a WebAssembly module to be insecure. I will need to take a deeper look into how to fix this.

@chrisknepper
Copy link
Owner

@cswartzvi I appreciate your patience on this one. It is frustrating for me as a developer to have this break so soon after I implemented it as a feature. This breaking is a result of a combination of:

  1. Google making a security change on their end (arguably a good one)
  2. The weird way dependencies from node_modules (like our electron-hunspell) library get loaded when required in a <webview> preload script, which is what we do.

I believe I have this fixed locally and I can explain here more tomorrow and hopefully get a release out soon.

@cswartzvi
Copy link
Author

cswartzvi commented Feb 25, 2019

I completely understand, and I appreciate the work you're doing. I am interested to hear about the (possible) solution. Please let me know I you need additional testers.

@chrisknepper
Copy link
Owner

chrisknepper commented Feb 27, 2019

Alrighty, here's the explanation as to why this is happening.

Recently, around the time I released v0.8.0, messages.android.com started sending the content-security-policy header. This header restricts the source URLs from which JavaScript in the page can be executed but can also prevent certain specific "unsafe" JavaScript operations from executing.

electron-hunspell uses WebAssembly to implement Hunspell. Explaining briefly, this gives it the ability to be nearly as fast as native compiled code (such as C++) but without having to compile in a particular OS ahead-of-time. However, because WebAssembly is much lower-level, it is considered more of a security risk and thus compiling or instantiating a WebAssembly module (.wasm) is forbidden by what Google is sending in the content-security-policy header.

Now this shouldn't be a problem because electron-hunspell is a dependency of this app, not messages.android.com. But it turns out, Electron, for whatever reason, loads local dependencies included in a <webview> preload as if they come from the src of the <webview> instead of from the local filesystem. So electron-hunspell looks like its coming from messages.android.com/node_modules instead of file://<install path>/node_modules and thus is incorrectly given the content-security-policy header which prevents it from instantiating. So it throws an exception. So spell check then doesn't work.

😐

I'm not sure why or if this behavior of pretending dependencies in a <webview> preload script come from its src can be changed, but it's definitely strange and, in my opinion, not good.

Luckily, Electron (well, actually Chromium) provides a way to deal with this problem. The headers sent to a <webview> can be intercepted and modified via the session.webRequest.onHeadersReceived callback (docs, MDN) which, when setup, lets us read/modify/delete headers before they are interpreted by the <webview>.

I'm traveling at the moment but should have a release out in the next day or two.

Additional resources:

https://developers.google.com/web/fundamentals/security/csp/
WebAssembly/content-security-policy#7
electron/electron#15310
electron/electron#12909
https://discuss.atom.io/t/electron-intercept-http-request-response-on-browserwindow/21868/8
https://stackoverflow.com/questions/48523118/wasm-module-compile-error-in-chrome-extension

chrisknepper added a commit that referenced this issue Mar 3, 2019
….android.com to fix WebAssemby compilation/spell check (Fixes #135)

-Add explicit options to app renderer BrowserWindow (per Electron warnings)
-Update package-lock
@vrl2
Copy link

vrl2 commented Jul 31, 2019

Hello, the spell check is no longer working on version 2.0.0 - running Chrome 75.0.3770.142 on MacOS. Will this be permanently fixed at some point?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants