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

Bug: calling editor.destroy() throws javascript error #375

Closed
brunobasto opened this issue Nov 13, 2015 · 4 comments
Closed

Bug: calling editor.destroy() throws javascript error #375

brunobasto opened this issue Nov 13, 2015 · 4 comments
Labels
Milestone

Comments

@brunobasto
Copy link

@brunobasto brunobasto commented Nov 13, 2015

Hey guys,

Found a small issue. To reproduce, try this: brunobasto@9b71aff

Thanks,

@brunobasto brunobasto added the bug label Nov 13, 2015
@brunobasto brunobasto changed the title Calling editor.destroy throws javascript error Bug: calling editor.destroy throws javascript error Nov 13, 2015
@brunobasto brunobasto changed the title Bug: calling editor.destroy throws javascript error Bug: calling editor.destroy() throws javascript error Nov 13, 2015
@ipeychev
Copy link
Contributor

@ipeychev ipeychev commented Nov 14, 2015

Hey @brunobasto,

Thanks for the report! The exception is not in our code, but in CKEditor's engine itself. It seems they don't like when you create an instance of the editor and then immediately destroy it.
This is not a critical bug, since normally the editor won't be destroyed immediately after its creation.
Keeping that in mind, we will have to upgrade to newer version of CKEditor (if any), or to report it and wait for a a fix.

Thanks,

@brunobasto
Copy link
Author

@brunobasto brunobasto commented Nov 15, 2015

Got it! Thanks, @ipeychev

@ipeychev
Copy link
Contributor

@ipeychev ipeychev commented Nov 21, 2015

Hey @brunobasto,

It seems in CKEditor v4.4.5 this is fixed, they now throw a warning (http://docs.ckeditor.com/#!/guide/dev_errors-section-editor-incorrect-destroy). @jbalsas could upgrade to this version and this upgrade will fix #381 too.

Thanks,

ipeychev added a commit that referenced this issue Nov 24, 2015
@brunobasto
Copy link
Author

@brunobasto brunobasto commented Nov 24, 2015

Thanks, guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants