Opened 13 years ago
Closed 13 years ago
#6389 closed defect (fixed)
[patch][cla] _removeItems() in DojoData needs to check for existence of data[i]
Reported by: | guest | Owned by: | benschell |
---|---|---|---|
Priority: | high | Milestone: | 1.2 |
Component: | DojoX Grid | Version: | 1.0 |
Keywords: | 1.0.2 release | Cc: | [email protected]… |
Blocked By: | Blocking: |
Description (last modified by )
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); } }
Attachments (2)
Change History (10)
comment:1 Changed 13 years ago by
Description: | modified (diff) |
---|---|
Owner: | changed from sorvell to benschell |
comment:2 Changed 13 years ago by
Description: | modified (diff) |
---|---|
Status: | new → assigned |
Changed 13 years ago by
Attachment: | 6389.patch added |
---|
comment:3 Changed 13 years ago by
comment:4 Changed 13 years ago by
Summary: | _removeItems() in DojoData needs to check for existence of data[i] → [patch][cla] _removeItems() in DojoData needs to check for existence of data[i] |
---|
comment:5 Changed 13 years ago by
Milestone: | → 1.2 |
---|
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)
comment:7 Changed 13 years ago by
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.
Changed 13 years ago by
Attachment: | 6389-2.patch added |
---|
comment:8 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Patch uploaded. Corrects the behavior and includes a new test case to facilitate testing larger result sets with DojoData?.