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

Browser-Addon LanguageTool: one-click-correction stopped working with 35.3.0 #12824

Closed
TobiasHartlehnert opened this issue Nov 9, 2022 · 16 comments
Labels
domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). resolution:resolved This issue was already resolved (e.g. by another ticket). squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@TobiasHartlehnert
Copy link

TobiasHartlehnert commented Nov 9, 2022

📝 Provide detailed reproduction steps (if any)

  1. Install and activate browser-addon "LanguageTool" (Chrome Webstore, Firefox Addons)
  2. Navigate to Classic Editor example
  3. Click inside the editing area; LanguageTool will underline "earth" in first paragraph as a possible error
  4. Click anywhere inside "earth" to open the LanguageTool correction box
  5. Click on the suggested correction "earth," (blue button)

✔️ Expected result

"earth" should change to "earth,"

❌ Actual result

Text is not changed. JS error is thrown, see Console (Chrome: "Uncaught TypeError: Cannot read properties of null (reading 'root') ...", Firefox: "Uncaught CKEditorError: i is null ...").

📃 Other details

  • Browser: Chrome 107.0.5304.106, Firefox 106.0.5
  • OS: Windows 10 Pro
  • First affected CKEditor version: 35.3.0

We had to downgrade to CKEditor5 35.2.1, because a lot of our clients are using the LanguageTool. Until this is fixed, we can't upgrade to any future version of CKEditor5 :(

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@TobiasHartlehnert TobiasHartlehnert added the type:bug This issue reports a buggy (incorrect) behavior. label Nov 9, 2022
@niegowski
Copy link
Contributor

It looks like this browser extension is using beforeInput:insertText but it is not providing targetRanges this might be easy to fix by enabling fallback to the DOM document selection.

@niegowski niegowski added domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). squad:core Issue to be handled by the Core team. labels Nov 9, 2022
@Reinmar
Copy link
Member

Reinmar commented Nov 10, 2022

Could you also open issue to LanguageTool itself? Even if we can workaround this issue today, let's help them fix this so we have a cleaner situation in the future.

@TobiasHartlehnert
Copy link
Author

Could you also open issue to LanguageTool itself? Even if we can workaround this issue today, let's help them fix this so we have a cleaner situation in the future.

Sure, they also have a github repo for that. Can you provide me with some description/hint for the issue, to give them a better understanding where to start?

@tiff
Copy link

tiff commented Nov 15, 2022

I'm one of the developers behind LanguageTool. First, thanks for being helpful and constructive.

@niegowski it seems there's no way for us to set “target ranges” when simulating the beforeinput event. At least, I'm not aware of any viable approach. If you are, I'd be happy if you could point me towards the corresponding documentation.
What do you mean by enabling the document selection fallback? I guess that's something you would have to do on your end?
In general, we are also in favor of solving this on our end and are already investigating this.

@tiff
Copy link

tiff commented Nov 16, 2022

It seems that simply setting targetRanges in the second parameter of new InputEvent(...) fixes it. We will begin rolling out a fix next week. Feel free to close this issue.

@Reinmar
Copy link
Member

Reinmar commented Nov 16, 2022

Wow, that was fast :) Thanks @tiff!

Do you think we could help testing it with CKE5? Is it possible to check it out before your release? :)

@tiff
Copy link

tiff commented Nov 16, 2022

Hey @Reinmar unfortunately, we do not publish release candidates, but I will notify you once the update is available and promise that we will investigate further problems and regressions ✌️

@CKEditorBot CKEditorBot removed the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Nov 16, 2022
@tiff
Copy link

tiff commented Nov 23, 2022

@Reinmar an update for our Chrome extension (incl. this fix) has been released. Please make sure you have the latest version (6.0.0) installed. Firefox, Edge, and Safari will follow in the next days.

@TobiasHartlehnert
Copy link
Author

@tiff works for me in Chrome. Thanks for the quick fix!

@Reinmar
Copy link
Member

Reinmar commented Nov 23, 2022

Confirmed by our team as well. Thank you!

@Reinmar Reinmar closed this as completed Nov 24, 2022
@Reinmar Reinmar added the resolution:resolved This issue was already resolved (e.g. by another ticket). label Nov 24, 2022
@jensgerdes-dpa
Copy link

It looks like this browser extension is using beforeInput:insertText but it is not providing targetRanges this might be easy to fix by enabling fallback to the DOM document selection.

We are experiencing the same issue after having upgraded CKEditor with LanguageTool. The LT support has already been contacted to provide us with an update. However, we would very much appreciate if there is the possibility for a quick hotfix in the meantime.

Is "enabling fallback" a configuration option?

@Witoso
Copy link
Member

Witoso commented Apr 27, 2023

@jensgerdes-dpa to which version have you upgraded? I have not noticed issues with LT enabled in our docs.

@jensgerdes-dpa
Copy link

@Witoso from 32.0.0 to the current one (37.1.0)

@tiff
Copy link

tiff commented Apr 27, 2023

@jensgerdes-dpa are you using our (LanguageTool's) browser add-on or our JavaScript library?

@Witoso
Copy link
Member

Witoso commented Apr 27, 2023

And what problems do you observe?

Screen.Recording.2023-04-27.at.12.39.30.mov

@jensgerdes-dpa
Copy link

@Witoso When applying suggestions from LanguageTool like below

applying-suggestion

we get the following Error in the JS console.

js-error

@tiff We are using your JS Library in combination with the self-hosted premium version of LanguageTool. For that purpose we received a JS file some years ago. This one - like with the browser add-on - does not provide a targetRange which causes the CKEditor to crash.

While waiting for the LT support to send me an update I wondered if the change @niegowski mentioned could be a first hotfix: enabling fallback to the DOM document selection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:typing/ime This issue reports a problem with standard typing & IME (typing method for CJK languages). resolution:resolved This issue was already resolved (e.g. by another ticket). squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

7 participants