[7.1.1] Can't set data in a readOnly cell if there is any selection

[aarondfrancis]

Description

You should be able to set data in a readOnly cell when any cell is selected, but you can't. If a cell is selected, HoT appears to try to validate something and then errors out with Cannot read property 'removeEventListener' of undefined

Steps to reproduce

1. Go to fiddle

Demo

https://jsfiddle.net/dgh4f6wp/3/

Your environment

• Handsontable version: 7.1.1
• Browser Name and version: Chrome
• Operating System: OS X

[aarondfrancis]

I think I've traced it to this particular PR: https://github.com/handsontable/handsontable/pull/6021/files.

Specifically, https://github.com/handsontable/handsontable/pull/6021/files#diff-7eb52b366866677666470e019283c8eaR926 and https://github.com/handsontable/handsontable/pull/6021/files#diff-7eb52b366866677666470e019283c8eaR947.

I think the issue is that when the selection is on a readOnly cell and you try to update any other cell, then the activeEditor returns the editor of the readOnly cell. I think the bigger issue is that the editor should really be the editor that goes along with the cell thats being updated, not necessarily the active editor.

If I have my selection on a DateCell and something calls setDataAtRowProp in the background on a TextCell, I think the editor used in that method will be date, instead of text.

[aarondfrancis]

Here's my full monkey patch, for anyone interested. Temporarily overwriting getActiveEditor fixed this issue, but it's not pretty and I don't think it'd work if you were using the HoT client-side validation.

/**
 * Monkey patch anything we want on the HoT instance
 * @param hot
 */
function monkey(hot) {
    const setDataAtRowProp = hot.setDataAtRowProp;
    const setDataAtCell = hot.setDataAtCell;
    const getActiveEditor = hot.getActiveEditor;

    // https://github.com/handsontable/handsontable/issues/6208
    hot.setDataAtRowProp = function (row, prop, value, source) {
        return safeSetData(setDataAtRowProp, row, prop, value, source)
    }

    // https://github.com/handsontable/handsontable/issues/6208
    hot.setDataAtCell = function (row, column, value, source) {
        return safeSetData(setDataAtCell, row, column, value, source)
    }

    function safeSetData(fn, ...args) {
        const editor = hot.getActiveEditor();
        let restore = null;

        // See if there is an editor in a currently editing state
        if (editor && editor.state !== 'STATE_VIRGIN') {
            // If there is, then grab the value
            restore = editor.getValue();
        }

        // Because of the Github issue referenced below, we need to
        // temporarily kill the getActiveEditor method. This is only
        // feasible because we do all of our validation server side.
        // Should remove this once the bug is addressed.
        // https://github.com/handsontable/handsontable/issues/6214
        hot.getActiveEditor = () => null;

        // Call the core function to set the data
        fn(...args);

        // Restore the original method
        hot.getActiveEditor = getActiveEditor;

        // If there was a value being edited, put it back
        if (restore !== null) {
            editor.setValue(restore);
        }
    }

    // We do all of our validation server side.
    // Also: https://github.com/handsontable/handsontable/issues/6214
    hot.validateCell = () => false;

    hot.getCellValidator = () => null;

    return hot;
}

[AMBudnik]

Hey @aarondfrancis

Thank you for sharing is an interesting bug and doing the whole investigation! I have simplified the example, even more, https://jsfiddle.net/6hu9v845/ but the bug is still present.

Tested with Chrome 76/ Windows 10.
Confirmed, breaks in the latest patch.

[aarondfrancis]

Awesome, thanks for boiling it down even further! Looking forward to a fix.

[kukchanka]

I have the same issue with handsontable v.7.1.1.
Error occurs because handsontable tries to close editor box of readonly cell and remove all event listeners it creates for active editor input, but since no editor element was created it cannot trigger removeEventListener of non-existing element. The activeEditor methods call chain looks like this:
activeEditor.cancelChanges =>
activeEditor.discardEditor =>
activeEditor.close
the last one calls unObserve method which tries to remove eventListeners from non-existing element and fails

The patch I use in my code is

export function applyActiveEditorMonkeyPatch(hotInstance) {
  const activeEditor = hotInstance.getActiveEditor();
  activeEditor.autoResize.unObserve = () => {};
}

[budnix]

Connected with #6246

[aninde]

@aarondfrancis this bug will be fixed in 7.1.2 7.2.0 version. You will be able to set data in a cell with readOnly when any cell is selected.

The new version will be released next week.

Results
Original case

Simplified case

Demo:
original case v. 7.1.2 7.2.0 https://jsfiddle.net/aninde/5j2Lkzd4/
simplified example v. 7.1.2 7.2.0 https://jsfiddle.net/aninde/x1c4ayo7/

Environment
Handsontable version: 7.1.2 7.2.0
Browser Name and version: any
Operating System: any

[aarondfrancis]

Thanks @aninde!

[AMBudnik]

That was an interesting bug but I'm glad it is gone ;D Aaron - we've just released a fresh new version. Enjoy

[AMBudnik]

@kukchanka please update to 7.2.0 and let me know if it works for you as well.

[aarondfrancis]

Awesome, thanks yall!