Opened 15 months ago

Last modified 5 months ago

#14613 review Bug

race issue loading plugins when editor already destroyed

Reported by: Brian Owned by: Tade0
Priority: Normal Milestone:
Component: General Version:
Keywords: 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 15 months ago.
Simple html to async load ckeditor, inject and destroy it

Download all attachments as: .zip

Change History (6)

Changed 15 months ago by Brian

Attachment: index.html added

Simple html to async load ckeditor, inject and destroy it

comment:1 Changed 15 months 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 7 months ago by Tade0

Status: newconfirmed
Version: 4.5.94.5.0

comment:3 Changed 7 months ago by Jakub Ś

Keywords: race removed
Resolution: duplicate
Status: confirmedclosed
Version: 4.5.0

This issue is in fact a duplicate of #11502.

comment:4 Changed 5 months ago by Tade0

Resolution: duplicate
Status: closedreopened

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 5 months ago by Tade0

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