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

ckEditor editor.js leaking 'load' eventListener attached to the window object #4273

Closed
rohit1 opened this issue Sep 14, 2020 · 2 comments · Fixed by #4272
Closed

ckEditor editor.js leaking 'load' eventListener attached to the window object #4273

rohit1 opened this issue Sep 14, 2020 · 2 comments · Fixed by #4272
Assignees
Labels
core The issue is caused by the editor core code. status:confirmed An issue confirmed by the development team. type:bug A bug.
Milestone

Comments

@rohit1
Copy link
Contributor

rohit1 commented Sep 14, 2020

Type of report

in ckEditor_base, we register 'load' event listener to window object, but never remove it

See below -

if ( document.addEventListener ) {
window.addEventListener( 'load', onReady, false );

if you see in destroy function at removeAllListeners()

this.removeAllListeners();

image

Probably we should remove this from window object under onReady() function at this try block where we detach others

if ( document.addEventListener ) {

document.removeEventListener( 'DOMContentLoaded', onReady, false );

image

Proposing the fix from this change -
rohit1:rohit1-ckeditor4-load-eventlistener-leak-fix

Raising a PR here for inputs on this proposed solution -
#4272

please advise if this is NOT the right way to fix this:)
thank you

Bug

Provide detailed reproduction steps (if any)

Expected result

What is the expected result of the above steps?

Actual result

What is the actual result of the above steps?

Other details

  • Browser: …
  • OS: …
  • CKEditor version: …
  • Installed CKEditor plugins: …
@rohit1 rohit1 added the type:bug A bug. label Sep 14, 2020
@Comandeer Comandeer added core The issue is caused by the editor core code. status:confirmed An issue confirmed by the development team. labels Sep 14, 2020
@Comandeer
Copy link
Member

Good catch! It seems that we really don't remove that load listener.

@Comandeer
Copy link
Member

Merged in 170451 and will be released in CKEditor 4.15.1.

@f1ames f1ames added this to the 4.15.1 milestone Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core The issue is caused by the editor core code. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants