Opened 7 years ago

Closed 7 years ago

#15810 closed defect (patchwelcome)

Using rowHeight and filtering cause blank page to appear at the end of grid

Reported by: Sébastien Le Ray Owned by: Evan
Priority: undecided Milestone: tbd
Component: DojoX Grid Version: 1.7.3
Keywords: Cc:
Blocked By: Blocking:

Description

OK, I've tracked down the bug and will try to explain the what and why as precisely as possible.

If you create a DataGrid? with rowHeight set and an initial pageCount of zero, then setQuery to have a larger page count, you'll get an additionnal (blank) page at the end of the grid (I only checked with initial page count being 0 but I guess applying a filter leading to 0 rows and then unfiltering does the same thing).

Now comes the explanation (line numbers come from 1.5.1 but I checked on SVN and code is the same):

When you have 0 pages, _Scroller::updatePageHeight (~l.298) is called with an inPageIndex of 0 for the first page even if this page does not exists since there are zero pages. It in turn calls measurePage(~l.160) to get the actual size of the page. Here is the bug, inPageIndex is invalid but if you do not define rowHeight, return (n && n.innerHTML) ? n.offsetHeight : undefined; will return undefined and all will be fine but if rowHeight is defined, it returns 0, this value is then cached as the size of the first page (because inPageIndex is 0).

Now, let's imagine you apply another filter causing pageCount to be non-zero.

You hit _Scroller::updateRowCount, especially this.height += this.defaultPageHeight * (this.pageCount - oldPageCount - 1) + this.calcLastPageHeight(); (~l.152), which will set container height to defaultSize * (pageCount - 1) + lastPageHeight (since oldPageCount was zero), which is precisely the size you needed. Things are going tricky now, since DataGrid? code is optimized it will resize each page based on its previous size using… the cache that has been previously corrupted. So updatePageHeight(~l.300) will see that page 0 was previously 0 (oh) and is now rowHeight * rowCount (h) and will add h - oh to the container height which lead to an additional blank page. Without rowHeight set, the cache hasn't been corrupted and h is set to undefined which causes it to take the value of oh and nothing is added.

There are two quick fixes (beside fixing the this.page value which seem to be more intrusive), either delete this.pageHeights[0]; in updateRowCount when oldPageCount is 0 (this.getPageHeight will return undef and oh = h) or fix measurePage to return undefined if(this.rowCount == 0).

Attachments (2)

dojox-15810.patch (596 bytes) - added by Sébastien Le Ray 7 years ago.
A possible (the simplest) fix (without color codes this time…)
dojox-15810.2.patch (619 bytes) - added by Sébastien Le Ray 7 years ago.
Added guard for rowCount == 0 or "beyond last page" case

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by Sébastien Le Ray

Same kind of issue when sorting with QueryReadStore? if you first scroll beyond first page.

This time this is due to measurePage returning negative value because rowCount is set to zero during sorting (sort calls _clearData). Adding if(inPageIndex > this.pageCount) return undefined; fixes this case

Changed 7 years ago by Sébastien Le Ray

Attachment: dojox-15810.patch added

A possible (the simplest) fix (without color codes this time…)

comment:2 Changed 7 years ago by Sébastien Le Ray

I added a patch that seems to fix both issues (patch is against r29443)

Changed 7 years ago by Sébastien Le Ray

Attachment: dojox-15810.2.patch added

Added guard for rowCount == 0 or "beyond last page" case

comment:3 Changed 7 years ago by Sébastien Le Ray

The whole mess seems to come for rowCount == 0, inPageIndex test should be useless as far as I can see

comment:4 Changed 7 years ago by bill

DojoX Grid and EnhancedGrid are deprecated in favor of dgrid and gridx.

You should upgrade your code to use one of those two grids.

We will consider patches to the old DojoX Grid code though.

comment:5 Changed 7 years ago by bill

Resolution: patchwelcome
Status: newclosed
Note: See TracTickets for help on using tickets.