Ext.view.NodeCache must confirm element exists before trying to use it

[brian428]

Using a grid with BufferedRenderer, if an item is deliberately removed from the backing store and that item's index isn't in the current set of viewable indexes, an error is thrown during NodeCache.removeElement().

Since the index falls within the range of fromIndex and endIndex, it tries to locate the element and run el.setAttribute('data-recordIndex', index) on it. This neglects the fact that even though the index is within fromIndex and endIndex, it is NOT present in the more limited set of indexes in the elements object.

I'm not sure if this means fromIndex and endIndex need to be changed to reflect only the indexes that are actually rendered, or if a check just needs to be made prior to the setAttribute() call to confirm an element was found. In my case, I manually changed the condition on line 272 of NodeCache from

if (fromIndex <= me.endIndex)

to

if (fromIndex <= me.endIndex && elements.hasOwnProperty(index))

This gets around the problem and everything seems to work correctly. I'm also not sure if there are other views that may be susceptible to this issue, or if NodeCache is the only view that could be affected by using the BufferedRenderer.

[mitchellsimoens]

I opened up the buffer-grid.html example and used this in the console with no error:

Ext.ComponentQuery.query('gridpanel')[0].getStore().removeAt(4000);

Record at 4,000 index isn't in view as I haven't scrolled at all. Did I miss a step?

[brian428]

Yes, looks like it has to do with actually scrolling. For example, if you scroll all the way to the end of the grid, then try to run Ext.ComponentQuery.query('gridpanel')[0].getStore().removeAt(0);, you'll get the error.

[brian428]

In case it narrows down the cause, a bit more playing seems to indicate that it ONLY happens if you scroll all the way to the end and then try to remove an item that's above the top of the viewable list.

[mitchellsimoens]

Yeah, seems the scrolling is the key. Will report this into our bug system.

[Animal]

What if you add the following override?

Ext.override(Ext.grid.plugin.BufferedRendererTableView, {

    onRemove: function(store, records, indices) {
        var bufferedRenderer = this.bufferedRenderer;

        // The view has the full complement of rows that the BufferedRenderer needs - must refresh
        if (bufferedRenderer && this.all.getCount() >= bufferedRenderer.viewSize) {

            // If the removed range is entirely outside the rendered range, we don't care about removal.
            if (indices[0] < this.all.startIndex || indices[indices.length - 1] > this.all.endIndex) {
                return;
            }

            this.onDataRefresh();
        }
        // No BufferedRenderer present
        // or
        // View has not yet reached the viewSize: we can add as normal.
        else {
            this.callParent([store, records, indices]);
        }
    }
});

[Animal]

No, that's no good. It must update the view on delete to bump the data upwards if a row above has been deleted...

Hmm...

[Animal]

I can't make it throw an error.

I run examples/grid/buffer-grid.html

Then get a reference to the store, and scroll the data to the end.

Then do

store.removeAt(0);

It goes to row zero, so the preserveScrollOnRefresh needs to be set if buffered rendering is happening (I'll fix that)

But it does not throw an error. How can I reproduce this?

[brian428]

Animal, are you saying if you do exactly what I did above (in the browser console), you don't get an error?

[Animal]

Yep!