Opened 4 years ago

Last modified 2 years ago

#14613 review Bug

race issue loading plugins when editor already destroyed

Reported by: Brian Owned by: Tade0
Priority: Normal Milestone:
Component: General Version: 4.5.0
Keywords: race Cc:

Description

See end of description for suggested fix.

Steps to reproduce

Find attached index.html which loads CKEditor script async and creates instance of ckeditor before quickly destroying the instance. To repo, place index.html in a directory along with the root ckeditor folder containing the source.

Directory should look like: /

index.html ckeditor ckeditor.js

Load index.html and open chrome developer tools. Press button (sometimes takes a few times). You will hopefully see ckeditor.js:formatted:16963Uncaught TypeError: Cannot read property 'customConfig' of undefined

Expected result

Race conditions should be handled accordingly.

Actual result

Plugin tries to load after editor.status === 'destroyed' and is unable to reference the editor instance.

Other details (browser, OS, CKEditor version, installed plugins)

My guess for the fix is in core/editor.js:596 the plugin loader's call back should check if editor.status === 'destroyed' and simply return. However there may need to be more clean up but this is my simple fix to prevent us from running into this issue. Maybe you all will have a more elegant solution :)

Attachments (1)

index.html (1.1 KB) - added by Brian 4 years ago.
Simple html to async load ckeditor, inject and destroy it

Download all attachments as: .zip

Change History (5)

Changed 4 years ago by Brian

Attachment: index.html added

Simple html to async load ckeditor, inject and destroy it

comment:1 Changed 4 years ago by Brian

Oops, looks like my formatting did not turn out for the directory structure,

root should have index.html and a ckeditor folder. Inside the ckeditor folder contains the sources (IE ekeditor.js)

comment:2 Changed 3 years ago by Tade0

Status: newconfirmed
Version: 4.5.94.5.0

comment:4 Changed 3 years ago by Tade0

Status: confirmedreopened

Added a unit test to the associated pull request: https://github.com/ckeditor/ckeditor-dev/pull/273

Rebased with major.

Changes pushed to branch:t/14613

comment:5 Changed 3 years ago by Tade0

Owner: set to Tade0
Status: reopenedreview
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy