CKEDITOR.ui.dialog.checkbox.setValue() should be chainable, but it actually isn't

[LordPachelbel]

The documentation says CKEDITOR.ui.dialog.checkbox.setValue() is a chainable method, just like all of the the other CKEDITOR.ui.dialog.*.setValue() methods for the other form field types, but when I try to use it as a chainable method I get an "Uncaught TypeError" exception in the console.

Here's my onChange handler for a select box in a dialog which messes with some other select boxes and a checkbox; it has been stripped down to just the relevant portions of the code:

var typeSelectionChanged = function() {
    var dialog            = this.getDialog(),
        typeValue         = this.getValue(),
        tableSelection    = dialog.getContentElement( 'info', 'tableSelection' ),    // <select>
        groupSelection    = dialog.getContentElement( 'info', 'groupSelection' ),    // <select>
        rowSelection      = dialog.getContentElement( 'info', 'rowSelection' ),      // <select>
        fieldSelection    = dialog.getContentElement( 'info', 'fieldSelection' ),    // <select>
        headingsSelection = dialog.getContentElement( 'info', 'headingsSelection' ); // <input type="checkbox">

    if(typeValue === 'group') {
        tableSelection.setValue('', true).disable();    // chaining works here
        groupSelection.enable();
        rowSelection.setValue('', true).disable();      // chaining works here
        fieldSelection.setValue('', true).disable();    // chaining works here
        headingsSelection.setValue('', true).disable(); // chaining DOES NOT work here -- "Uncaught TypeError: headingsSelection.setValue(...) is undefined"
    }

    dialog.layout();
};

I added some console debug statements just above the line that causes the exception:

console.debug(fieldSelection.setValue.toString());
console.debug(headingsSelection.setValue.toString());

And here is the result:

> function(a,b){this.getInputElement().setValue(a);!b&&this.fire("change",{value:a});return this}
> function(b,a){this.getInputElement().$.checked=b;!a&&this.fire("change",{value:b})}

Because there's no return this in the checkbox's version of the method, it's not actually chainable.

[LordPachelbel]

I found the method source at

ckeditor4/plugins/dialogui/plugin.js

Line 1201 in 5fe0590

setValue: function( checked, noChangeEvent ) {

:

/**
 * Sets the state of the checkbox.
 *
 * @param {Boolean} checked `true` to tick the checkbox, `false` to untick it.
 * @param {Boolean} noChangeEvent Internal commit, to supress `change` event on this element.
 */
setValue: function( checked, noChangeEvent ) {
    this.getInputElement().$.checked = checked;
    !noChangeEvent && this.fire( 'change', { value: checked } );
},

(By the way, the documentation's "View Source" link for the checkbox's setValue() method links to a different place, to the definition for CKEDITOR.ui.dialog.uiElement.prototype.setValue(), which is why I couldn't find the actual code at first and used that .toString() trick to output the actual source code to the browser console.)

[LordPachelbel]

The radio button version of setValue() also doesn't return anything, so it has the same problem.

If I'm understanding the code correctly, most of the form fields rely on the prototype version of setValue() and only the checkbox, radio button, and textInput classes have overridden versions of it. And the textInput version invokes the prototype version anyway after it does its custom stuff with return CKEDITOR.ui.dialog.uiElement.prototype.setValue.apply( this, arguments );

I think the checkbox and radio classes' overridden methods are deliberately not calling the prototype version of setValue() because its code ultimately alters the DOMNode .value property, which doesn't make sense to do in this context for radio buttons and checkboxes. And so I think the way to make the overridden methods chainable is to simply add return this; at the end of each of them, but I'm not familiar enough with CKEditor's API or source code to be confident enough about that to create my own pull request to do so.

[LordPachelbel]

All that said, the obvious workaround I'm using right now is to simply un-chain the two methods:

headingsSelection.setValue('', true);
headingsSelection.disable();

[KarolDawidziuk]

Hi, @LordPachelbel

Thank you for your report and investigation.
I can confirm that issue.

Like you said, adding return at the end of setValue() method inside CKEDITOR.ui.dialog.checkbox should solve this issue.

...but I'm not familiar enough with CKEditor's API or source code to be confident enough about that to create my own pull request to do so.

If you want and have time, I encourage you to create a pull request with a fix for this issue. We will try to help in case you encounter any problems.
Please see this guide for more information: https://ckeditor.com/docs/ckeditor4/latest/guide/dev_contributing_code.html

[CKEditorBot]

Closed in #5241