http://gwt-code-reviews.appspot.com/1500803/diff/1/user/src/com/google/gwt/user/cellview/client/AbstractPager.java File user/src/com/google/gwt/user/cellview/client/AbstractPager.java (left):
http://gwt-code-reviews.appspot.com/1500803/diff/1/user/src/com/google/gwt/user/cellview/client/AbstractPager.java#oldcode112 user/src/com/google/gwt/user/cellview/client/AbstractPager.java:112: rangeChangeHandler = null; On 2011/07/26 03:11:14, andycheng wrote:
should remove this line
Done. I re-added the line so I could verify that the test was working, then forgot to remove it. http://gwt-code-reviews.appspot.com/1500803/diff/1/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java File user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java (right): http://gwt-code-reviews.appspot.com/1500803/diff/1/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java#newcode250 user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java:250: assertNull(pager.getDisplay()); On 2011/07/26 03:11:14, andycheng wrote:
this test does not seem to catch the original bug. should test rowCountChangeHandler == null as well
The next time we set the display, we would get an error because we call rowCountChangeHandler.removeHandler() twice. 1. Set display: rowCountChangeHandler added 2. Set display to null: rowCountChangeHandler removed, but not null 3. Set display: rowCountChangeHandler removed again because it is not null. Results in an error. I made the test more explicit, testing that the fields are null and that the handlers are correctly removed from the display. http://gwt-code-reviews.appspot.com/1500803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
