Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Using Scrollbar on OnDemandGrid with many rows Causes JavaScript Error #195

Closed
nicknisi opened this Issue Jun 8, 2012 · 10 comments

Comments

Projects
None yet
7 participants
Member

nicknisi commented Jun 8, 2012

When there are a large number of rows (I'm testing with 2000 rows) and the scrollbar is used to quickly scroll down, the following JavaScript error occurs:

TypeError: Cannot read property 'element' of undefined

This error occurs on line 58 of dgrid/Grid.js

In this instance, I believe that target should be a row in the grid, but instead it is the grid container itself.

<div class="manageInvoicesGrid ui-widget dgrid dgrid-grid" data-dojo-attach-point="gridContainer" id="dgrid_0" role="grid">

target is passed as a parameter in the following way:

var row = this.row(target);

row is undefined and this is causing the error.

If the grid is scrolled with the mousewheel, this never happens. It is only when the scrollbar is grabbed and then dragged quickly.

@ghost

ghost commented Jun 25, 2012

We're having trouble reproducing this issue (trying with test/performance.html which has 20k rows). If you're still experiencing this issue, would you happen to have a standalone test case that you could put in a gist for us? Also, what browser(s) do you see this problem in? Thanks.

Member

nicknisi commented Jun 26, 2012

I was able to reproduce this issue. I've seen it occur in the latest Chrome and Firefox and running on OS X Lion.

Using test/performance.html in its current form, I was not able to reproduce it.

In our implementation, one of our columns uses the editor plugin and loads in a dijit/form/CheckBox, so I added that to the test and was still unable to reproduce it. I increased the number of rows to 100,000 and still no error.

I tried switching the store to dojo/store/JsonRest and it appears that the combination of having an editor field and using JsonRest does reproduce the issue. I did try getting rid of the editor column and it seemed to be ok again. I have created a gist containing my modified version of performance.html and the simple server I used to return the data.

https://gist.github.com/2993553

I don't know if this is strictly related, but I was getting errors myself when scrolling fast on a large grid with editable fields. I noticed it was a server side error because the requested range was "negative". i.e. the store requested items "300-275" instead of "275-300". It only happened sometimes, and I don't know why dgrid would request it in inverse order, but check to see if this is what's happening.

Contributor

kriszyp commented Oct 4, 2012

I tried a number of different ways, and can't seem to reproduce this at all. I used the provided test case, just porting it to PHP and still didn't have any luck. I can't get the test case to trigger this error at all.

I was able to reproduce it too or found something similar...
(Using Dojo 1.8 and Crome 22, test/performance.html).

As nicknisi wrote, it happens when the scrollbar grabbed and dragged quickly.
After that, simply scrolling up and down, sooner or later the grid will make errors.

found two solution for that

1, In the List.js insertRow function:
Sometimes the previousRow points to a "dgrid-preload" dom object, which has no rowIndex property and the adjustRowIndices will modify the sibling rows rowIndex to NaN (undefined++).

so replace the conditon

if(previousRow){
  // in this case, we are ...
  this.adjustRowIndices(previousRow);
}

to

if(previousRow && ((previousRow.className + ' ').indexOf("dgrid-row ")>-1) ){
  // in this case, we are ...
  this.adjustRowIndices(previousRow);
}

2. Or set the grid queryRowsOverlap property to 0.
It works too, but i don't know why :)

Hope it helps.

mvaerle commented Nov 12, 2012

I'm having the same issue, can reproduce very consistently. I'm using a JsonRest store. The problem goes away when reverting back to 0.3.0.

@ghost

ghost commented Dec 3, 2012

Some of these issues sound potentially related to #323 and/or #318; both of those have fixes in master now. Can you guys check and see if you still have issues running against the latest dgrid code?

pecke01 commented Dec 18, 2012

We tried the latest and the only thing that worked for us was to go back to 0.3.0 that @mvaerle mentioned.

Member

nicknisi commented May 22, 2013

Using the latest everything (dgrid, dojo 1.9, etc.) I am still able to reproduce this bug. I have updated the gist with my reproducible example from earlier https://gist.github.com/nicknisi/2993553

Additionally, here is a video of the bug in action:

dgrid-bug

@ghost ghost pushed a commit to gratex/dgrid that referenced this issue May 22, 2013

@martinerko martinerko Fix #195: Ensure to destroy cell widget only when cellElement exists 6bd1c95
@ghost

ghost commented May 22, 2013

we were able to reproduce it as well and also fix it

@kfranqueiro kfranqueiro pushed a commit to kfranqueiro/dgrid that referenced this issue Jun 14, 2013

Kenneth G. Franqueiro Add test to reproduce #195 bee7c3b

@ghost ghost added a commit that referenced this issue Jul 10, 2013

Kenneth G. Franqueiro Add test to reproduce #195 220ac92

@ghost ghost closed this in e725d8e Jul 10, 2013

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment