ckeditor / github-writer

GitHub Writer - WYSIWYG Rich-Text Editor for GitHub, powered by CKEditor.
https://ckeditor.com/
Other
376 stars 61 forks source link

Writer instances are not destroyed while navigating inside GH #416

Closed Dumluregn closed 1 year ago

Dumluregn commented 1 year ago

📝 Provide detailed reproduction steps (if any)

  1. Build a dev version of the extension and load it in the browser.
  2. Open dev tools in the browser.
  3. Enter some page where Writer will load.
  4. Change the page by clicking some link, e.g. Issues tab.

✔️ Expected result

You should see the following (with the actual ID) in the console:

Editor id "${ this.id }" destroyed.

❌ Actual result

The editor is never destroyed. Some editor markup is leaking.

❓ Possible solution

The turbo:before-visit event that we listen on receives the HTML of the new page on navigation, not the old one. The old editors are removed from the DOM, but not destroyed, so they leak some markup and objects. We need to start listening on two other events: turbo:click for the "clicking" navigation and turbo:visit for the browser navigation (back and forward buttons).

📃 Other details

This is the root cause for https://github.com/ckeditor/github-writer/issues/375 occurs. The editors are never destroyed, so their references are kept in the Tooltip Manager. It prevents it from creating new tooltip wrapper instance on the consecutive navigation.


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