LGTM On Mon, Aug 8, 2011 at 4:42 PM, <jlaba...@google.com> wrote:
> Reviewers: rjrjr, > > Description: > Fixing a bug in HasDataPresenter where paging to a negative row index > causes an IndexOutOfRange exception. We now properly trim the keyboard > selected row to a non-negative value. > > Issue: 6383 > > > Please review this at > http://gwt-code-reviews.**appspot.com/1513804/<http://gwt-code-reviews.appspot.com/1513804/> > > Affected files: > M user/src/com/google/gwt/user/**cellview/client/**HasDataPresenter.java > M user/test/com/google/gwt/user/**cellview/client/** > HasDataPresenterTest.java > > > Index: user/src/com/google/gwt/user/**cellview/client/** > HasDataPresenter.java > ==============================**==============================**======= > --- user/src/com/google/gwt/user/**cellview/client/**HasDataPresenter.java > (revision 10507) > +++ user/src/com/google/gwt/user/**cellview/client/**HasDataPresenter.java > (working copy) > @@ -719,8 +719,9 @@ > } else if (KeyboardPagingPolicy.CHANGE_**PAGE == keyboardPagingPolicy) > { > // Go to previous page. > while (index < 0) { > - newPageStart -= pageSize; > - index += pageSize; > + int shift = Math.min(pageSize, newPageStart); > + newPageStart -= shift; > + index += shift; > } > > // Go to next page. > @@ -731,9 +732,10 @@ > } else if (KeyboardPagingPolicy.**INCREASE_RANGE == > keyboardPagingPolicy) { > // Increase range at the beginning. > while (index < 0) { > - newPageSize += PAGE_INCREMENT; > - newPageStart -= PAGE_INCREMENT; > - index += PAGE_INCREMENT; > + int shift = Math.min(PAGE_INCREMENT, newPageStart); > + newPageSize += shift; > + newPageStart -= shift; > + index += shift; > } > if (newPageStart < 0) { > index += newPageStart; > Index: user/test/com/google/gwt/user/**cellview/client/** > HasDataPresenterTest.java > ==============================**==============================**======= > --- > user/test/com/google/gwt/user/**cellview/client/**HasDataPresenterTest.java > (revision 10507) > +++ > user/test/com/google/gwt/user/**cellview/client/**HasDataPresenterTest.java > (working copy) > @@ -750,6 +750,18 @@ > view.**assertKeyboardSelectedRowEmpty**(); > assertEquals(10, presenter.getVisibleRange().**getStart()); > assertEquals(10, presenter.getVisibleRange().**getLength()); > + > + // Negative index out of range. > + presenter.setVisibleRange(new Range(3, 10)); > + presenter.**setKeyboardSelectedRow(3, false, false); > + populatePresenter(presenter); > + presenter.flush(); > + presenter.**setKeyboardSelectedRow(-4, false, false); > + populatePresenter(presenter); > + presenter.flush(); > + assertEquals(0, presenter.**getKeyboardSelectedRow()); > + assertEquals(0, presenter.getVisibleRange().**getStart()); > + assertEquals(10, presenter.getVisibleRange().**getLength()); > } > > public void testSetKeyboardSelectedRowCurr**entPage() { > @@ -880,6 +892,18 @@ > assertEquals(0, presenter.getVisibleRange().**getStart()); > pageSize += 10; > assertEquals(pageSize, presenter.getVisibleRange().**getLength()); > + > + // Negative index out of range. > + presenter.setVisibleRange(new Range(3, 10)); > + presenter.**setKeyboardSelectedRow(3, false, false); > + populatePresenter(presenter); > + presenter.flush(); > + presenter.**setKeyboardSelectedRow(-4, false, false); > + populatePresenter(presenter); > + presenter.flush(); > + assertEquals(0, presenter.**getKeyboardSelectedRow()); > + assertEquals(0, presenter.getVisibleRange().**getStart()); > + assertEquals(13, presenter.getVisibleRange().**getLength()); > } > > public void testSetRowCount() { > > > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
