ckeditor / ckeditor5-react

Official CKEditor 5 React component.
https://ckeditor.com/ckeditor-5
Other
426 stars 100 forks source link

Infinite loop while using the source editing plugin #391

Closed corymharper closed 1 year ago

corymharper commented 1 year ago

Disclaimer: I am not 100% sure if this is an issue of this package, or a different CKEditor package, this is just where I am starting.

Description

When you are using the source editing plugin, at least in React, if you make any changes in the source editing tab it can seemingly lead to infinite loops under certain circumstances. I am not very certain of the cause, but I am investigating it.

Reproduction

Warning: These steps cause an infinite loop in the browser

  1. Run yarn start locally after cloning this repository and visit the URL indicated by the command, or visit the Github Pages website.

  2. Open the browser console.

  3. Enter any text into the displayed editor.

  4. Switch to "source editing" mode using the toolbar.

  5. Make any change to the text in the editor now that source editing is active.

  6. The infinite loop may have already started, if it has you will see that displayed in the console, otherwise hit the submit button and it should start.

I used React Router for the reproduction because in my tests it was the most consistent way to reproduce the problem.

corymharper commented 1 year ago

Some updates regarding this issue:

  1. I think it happens specifically on watchdog/editor destruction
  2. I tried unsubscribing from the change:data event in the destructor method using setDestructor. This fixed the infinite loop problem but just opened up a different crash, where _save was being called in the Watchdog after the editor had been destroyed, leading to a runtime error from this line because the _editor value is null. Quite often the error arose because of this flush of the throttled save calls, but it was possible for it to arise from any call to _throttledSave after editor destruction.
  3. With the code I was using to unsubscribe from changes in this package on editor destruction, the crash would only happen if you destroyed the editor while in source editing mode. This makes me think that its somehow related to the source editing plugin's method of updating the editor data that happens only when editor.getData is called, which seems like a bad design to me, since the watchdog itself and many plugins may not be designed to handle getting the editors data causing that same data to update.

Right now I don't have a full proof fix, since if you add my code for unsubscribing in this package, you'll just run into errors in other CKEditor packages. With that in mind I'm also going to open this issue in the CKEditor 5 repo as well, since the fix might lie in changing how the source editing works. I'll still keep this issue open incase it turns out the solution is purely in this repository.

Since it might be important later, here is a patch file with my changes for unsubscribing: Changes-On-bd6d9a.patch

Witoso commented 1 year ago

Thanks for the report, we will take a look at this. Meanwhile, in the morning, there was a release of 6.1.0 with the possibility to disable the watchdog:

        <CKEditor
          disableWatchdog={true}
          editor={Editor}
          onChange={handleChange}
        />

AFAICS it prevents the infinitive loop so it may be a good workaround for the time being.

Witoso commented 1 year ago

We will close the issue here as we reproduced it without React. Follow the topic in the ckeditor/ckeditor5#14469.

corymharper commented 1 year ago

Thanks for the report, we will take a look at this. Meanwhile, in the morning, there was a release of 6.1.0 with the possibility to disable the watchdog:

        <CKEditor
          disableWatchdog={true}
          editor={Editor}
          onChange={handleChange}
        />

AFAICS it prevents the infinitive loop so it may be a good workaround for the time being.

Unfortunately this is not a workaround here, I tested it locally, I believe this is because the source of the bug is having a data change listener while using the source editing plugin, and outside of the watchdog, ckeditor5-react has its own change listener.

Witoso commented 1 year ago

I believe this is because the source of the bug is having a data change listener

Yep, we confirmed this in the ckeditor/ckeditor5#14469. This is not related to React. Somehow disabling Watchdog made it work locally for me but we will fix the root cause.

hyungrae94 commented 7 months ago

Hello !

By any chance... has this problem been solved?

corymharper commented 7 months ago

Hello !

By any chance... has this problem been solved?

Yes, you just need to be using a version of the source editing plugin 39.0.0 >=. No changes needed to ckeditor5-react. See https://github.com/ckeditor/ckeditor5/issues/14469.