Opened 12 months ago

Last modified 6 weeks ago

#14613 review Bug

race issue loading plugins when editor already destroyed

Reported by: brian.mcnamara Owned by: Tade0
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:


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.mcnamara 12 months ago.
Simple html to async load ckeditor, inject and destroy it

Download all attachments as: .zip

Change History (6)

Changed 12 months ago by brian.mcnamara

Simple html to async load ckeditor, inject and destroy it

comment:1 Changed 12 months ago by brian.mcnamara

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

  • Status changed from new to confirmed
  • Version changed from 4.5.9 to 4.5.0

comment:3 Changed 3 months ago by j.swiderski

  • Keywords race removed
  • Resolution set to duplicate
  • Status changed from confirmed to closed
  • Version 4.5.0 deleted

This issue is in fact a duplicate of #11502.

comment:4 Changed 6 weeks ago by Tade0

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Added a unit test to the associated pull request:

Rebased with major.

Changes pushed to branch:t/14613

comment:5 Changed 6 weeks ago by Tade0

  • Owner set to Tade0
  • Status changed from reopened to review
Note: See TracTickets for help on using tickets.
© 2003 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy