[patch][cla] _removeItems() in DojoData needs to check for existence of data[i]

[guest]

Upon deleting an item from a large store with identity (eg ItemFileWriteStore) the _rowIdenties are rebuilt. In case a user has scrolled through the grid fast enough, the data[] array can contain empty entries. Example: Grid with 20 rowsPerPage, 40 keepRows, 100 rows in total. User scrolls to the end fast, then the data[] contains entries for the first page data[0 ]- data[19 ] and the last page data[80 ] - data[99 ].

Rebuilding of the _rowIdenties has to check whether the data[i] entry exists, otherwise an error data[i].__dojo_data_item does not exist occurs.

File: dojox/grid/_data/model.js Class: dojox.grid.data.DojoData?

_removeItems: function(inRowIndexes /*array*/){
    // ...
    // Rebuild _rowIdentities
    this._rowIdentities = {};
    for (var i = 0; i < this.data.length; i++){
      // data[i] can be undefined ! need to add check
      if(this.data[i])
        this._setRowId(this.data[i].____dojo_data_item, 0, i);
    }
}

[benschell]

Patch uploaded. Corrects the behavior and includes a new test case to facilitate testing larger result sets with DojoData?.

[Adam Peller]

Ben, I'd be happy to commit the patch, but I need instructions on how to reproduce the bug with this test. Also, can you use dojo.forEach() in your patch? (the index should be available as the second arg)

[Adam Peller]

p.s. the test doesn't load. It says it can't find DojoData?

[benschell]

Adam,

I just uploaded another patch offering. I didn't change the for loop in the initial patch, I think I just cleaned up some of the spacing. This second patch has two differences from the first: a) changed the for(var i=0... to dojo.forEach(... as requested, and b) removed a comment in the test file.

I'm not sure why the test file wouldn't be loading. I applied the patch to a fresh checkout and it works fine for me, but I could be doing something wrong. The test file is essentially a direct copy of test_dojo_data_edit.html, with a change to which json file it is loading. Maybe I have a bug somewhere else?

As for how to reproduce the original bug, take the given test case (once it works). Grab the scroll handle on the top grid and scroll to the bottom as fast as you can, pause to let the data load, then scroll back to the top as quickly as possible. The goal is to have data loaded at the top and at the bottom, but some rows in between that are not loaded (this isn't as hard as it sounds). Next, select a row near the top (say, 3, for instance), then click the "Remove" button at the top. Without the patch, an error is thrown, as the row identities for those un-loaded rows in the middle of the grid are trying to be set. With the patch, those un-loaded rows are ignored, since they haven't been loaded from the store.

[Adam Peller]

(In [13392]) Check array elements in _removeItems. Fixes #6389. !strict Thanks, Ben.