Throw friendly error for invalid/missing id's on editor creation

[blzaugg]

Type of report

Feature request

Provide description of the new feature

Given the code below:

<textarea id="editor1"></textarea>

<script>
    CKEDITOR.replace('editor2');
</script>

The developer intended to target editor1, but by mistake targeted editor2, which doesn't exist in the DOM.

Doing this causes an error.

Uncaught TypeError: Cannot read property 'getEditor' of undefined

Without looking at the CKEditor source, it's not obvious what's wrong.

The Feature request is to throw a friendly error for invalid/missing id's.

Solution:

diff --git a/core/creators/inline.js b/core/creators/inline.js
index 61f88b2..d48af34 100644
--- a/core/creators/inline.js
+++ b/core/creators/inline.js
@@ -31,8 +31,15 @@
                if ( !CKEDITOR.env.isCompatible )
                        return null;

+               var elementArg = element;
+
                element = CKEDITOR.dom.element.get( element );

+               // Throw error on missing target element.
+               if ( !element ) {
+                       throw 'The provided element, "' + elementArg + '", is missing from the DOM.';
+               }
+
                // Avoid multiple inline editor instances on the same element.
                if ( element.getEditor() )
                        throw 'The editor instance "' + element.getEditor().name + '" is already attached to the provided element.';
diff --git a/core/creators/themedui.js b/core/creators/themedui.js
index 3b1b00b..814b815 100644
--- a/core/creators/themedui.js
+++ b/core/creators/themedui.js
@@ -323,8 +323,15 @@ CKEDITOR.replaceClass = 'ckeditor';
                if ( !CKEDITOR.env.isCompatible )
                        return null;

+               var elementArg = element;
+
                element = CKEDITOR.dom.element.get( element );

+               // Throw error on missing target element.
+               if ( !element ) {
+                       throw 'The provided element, "' + elementArg + '", is missing from the DOM.';
+               }
+
                // Avoid multiple inline editor instances on the same element.
                if ( element.getEditor() )
                        throw 'The editor instance "' + element.getEditor().name + '" is already attached to the provided element.';

[jacekbogdanski]

Hello,

this is the kind of the issue which I prefer to label as a bug rather than a feature cause it's more like missing implementation. @blzaugg as I see you already wrote some code, would you like to create PR for the issue?

[blzaugg]

Sure. I'll send a PR tomorrow.

[mlewand]

@jacekbogdanski it is actually a more of new feature rather than bugfix. I'll revert the original issue type, and because of that also the PR will need to be targetted at major branch instead. But Reviewer will handle this. Sorry for minor confusion 🙂

[mlewand]

Cool, the fix has been included and is staged to be released with 4.12.0. Thanks for the contribution @blzaugg!