search

electron-userland / electron-spellchecker

Implement spellchecking, correctly
MIT License
239 stars 83 forks source link

spellCheckHandler.attachToInput breaks on electron 5.0.0 #144

Closed robsonhermes closed 5 years ago

robsonhermes commented 5 years ago

Hello

Starting from Electron 5.0.0, the method spellCheckHandler.attachToInput breaks with:

image

I guess this happens cause parameters of method webFrame.setSpellCheckProvider changed in version 5.0.0 -> link.

smithalan92 commented 5 years ago

This is still an issue in 2.0.4

Looks like its a bigger issue than the setSpellCheckProvider signature changing though.

Correcting the passed params here and here , solves the processing error, but introduces a new error

image

mlalkaka commented 5 years ago

I'm working on fixing this. Will have a fork + PR up soon.

mlalkaka commented 5 years ago

@robsonhermes and @smithalan92, I've created a branch and PR to fix this. If you're still interested, could you try the change in your environments and/or provide feedback? The branch is mlalkaka/electron-spellchecker.

robsonhermes commented 5 years ago

@mlalkaka will try that in my first open slot and update you here. Many thanks!

smithalan92 commented 5 years ago

Thats great @mlalkaka , I gave that a test using Electron v5 and it works nicely 👌 Hopefully @felixrieseberg will have some time to approve the PR and deploy a new version

robsonhermes commented 5 years ago

Hm, I had the problem below.

Source code of my test is robsonhermes/electron-spellchecker-test

Just mind package.json, I used "electron-spellchecker": "0.0.1", cause this is how I built/published it locally. Did I miss anything? Not sure if this is somehow related to issue 148.

error
felixrieseberg commented 5 years ago

Fixed and released as 2.1.0 🙇

robsonhermes commented 5 years ago

Still not working for me.

Could anyone please tell me if you had the same problem?

https://github.com/robsonhermes/electron-spellchecker-test

npm install && .\node_modules\.bin\electron-rebuild.cmd
npm start

results in the error I posted in my last msg.

smithalan92 commented 5 years ago

Your example works fine for me @robsonhermes ( I'm on MacOS though )

robsonhermes commented 5 years ago

Just tried on Linux (Ubuntu). fails with the same error. But I guess it´s not related to this fix. Will open a separate issue.

Thanks for trying it out, @smithalan92

mlalkaka commented 5 years ago

@robsonhermes, sorry for the late reply here. I've been trying to repro this on Windows, but have been having unrelated troubles with my Windows PC. I'll give this another try later today.

On Thu, Jun 13, 2019, 6:20 AM Robson Hermes notifications@github.com wrote:

Just tried on Linux (Ubuntu). fails with the same error. But I guess it´s not related to this fix. Will open a separate issue.

Thanks for trying it out, @smithalan92 https://github.com/smithalan92

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/electron-userland/electron-spellchecker/issues/144?email_source=notifications&email_token=ACZ6REJ6PYRBUGRKG4NDULTP2JCRNA5CNFSM4HJLEMQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXTVHYY#issuecomment-501699555, or mute the thread https://github.com/notifications/unsubscribe-auth/ACZ6REKGTI36GEYYAWQBCMLP2JCRNANCNFSM4HJLEMQA .

robsonhermes commented 5 years ago

No problem @mlalkaka , I am the one to thank you for your efforts on fixing compatibility with Electron 5.

I believe this is a separate problem, thus decided to raise another issue to keep the discussion separated. Please see 151.