getCellFromEvent() not returning correct row when editing checkboxes with keyboard

[HobbsB]

Hi - my first issue created on GitHub. Please let me know if the issue is unclear or the write-up is not optimal.

Using 2.0-frozenRowsAndColumns.

When a checkbox cell is selected and editable, clicking spacebar triggers a click and marks the checkbox as selected.

This triggers the slick.grid.js function: handleClick(e)

Which calls getCellFromEvent(e)

In the standard Slickgrid, the value of the selected cell is returned from getCellFromEvent().

In this fork, the row that is returned is based on which row the idle mouse is located on the grid.

If a user is using the keyboard, this means that the next line: if (!cell || (currentEditor !== null && activeRow == cell.row && activeCell == cell.cell)) { return; }

will not execute the "return;", and will continue execution for an invalid click event.

This causes the onClick events to trigger for a cell which is not being edited or clicked.

[HobbsB]

I may very well be breaking other things with this solution: but modifying the function getRowFromNode to the following

    function getRowFromNode(rowNode) {

        for (var row in rowsCache) {
            if (rowsCache[row].rowNode[0] === rowNode || rowsCache[row].rowNode[1] === rowNode) {
                return row | 0;
            }
        }

        return null;
    }

and returning getCellFromEvent to it's previous implementation:

function getCellFromEvent(e) {
        var $cell = $(e.target).closest(".slick-cell", $canvas);
        if (!$cell.length) {
            return null;
        }
    var row = getRowFromNode($cell[0].parentNode);
    var cell = getCellFromNode($cell[0]);

        if (row == null || cell == null) {
            return null;
        } else {
            return {
                "row": row,
                "cell": cell
            };
        }
    }

fixes the issue.

Any thoughts?

[kavun]

getRowFromNode would be better as ...

function getRowFromNode(rowNode) {
    for (var row in rowsCache) {
        var rowCacheNode = rowsCache[row].rowNode;
        if (rowCacheNode && rowCacheNode.length) {
            for (var i = 0, l = rowCacheNode.length; i < l; i++) {
                if (rowCacheNode[i] === rowNode[0]) {
                    return row | 0;
                }
            }
        }
    }

    return null;
}

... in order to more gracefully handle the case where frozenColumns are not enabled. This way you don't have to hardcode the node indices.

[kavun]

Also, I believe you are correct in thinking that getCellFromEvent needs to use getRowFromNode instead of getCellFromPoint, but as far as I can tell, doing so will break event handling on frozen rows.

With the current implementation in 2.0-frozenRowsAndColumns, getCellFromEvent will return the wrong row for an event in IE8 if the browser zoom level is <100% (ie: 95%). This is because $.fn.offset() returns offset in screen pixels regardless of zoom and then is passed to getRowFromPosition which uses options.rowHeight to calculate the row index - which is off because of non-100% browser zoom.

I have no need to use frozenRows, so I have reverted to the original implementation of getCellFromEvent and setActiveCellInternal since both of those were using getRowFromPosition (which is broken in IE8 when zoomed out).