I suggest we use my patch: http://gwt-code-reviews.appspot.com/33816
Its the same as yours but includes test cases. Thanks, John LaBanca [email protected] On Thu, May 14, 2009 at 11:39 AM, Joel Webber <[email protected]> wrote: > David, > Good find. Looks like a straight-up mistake here. At the least, we should > do the same thing in setCellWidth/Height that we do in > setCellHorz/VertAlign. I don't see you here in > http://code.google.com/p/google-web-toolkit/source/browse/CLA-SIGNERS(though > if you *have* signed a CLA, please let me know so I can fix this), > so I'll go ahead and put a patch together. > > In the future, if you want to take care of these sorts of things quickly > (which I certainly wouldn't object to!), it would be helpful if you could > sign a CLA ( > http://code.google.com/webtoolkit/makinggwtbetter.html#committers) so we > can just take your patches directly. > > I just uploaded a patch here: http://gwt-code-reviews.appspot.com/35801 > Please have a look and make sure I didn't miss anything. > > Thanks, > joel. > > > On Thu, May 14, 2009 at 10:27 AM, stuckagain <[email protected]>wrote: > >> >> People, >> >> I just lost an hour or so trying to figure out why my application >> crashed in the browser and not in hosted mode. >> Unfortunately I was working with IE6 and when that one crashes you >> don't get a useful stacktrace. >> >> I finally tried my application in FireFox and found it. >> >> CellPanel.setCellHeight does not check that the given Widget is >> actually attached to the cellpanel. >> It gets the parent element and blindly assumes that it is a TD. In my >> case it was null. >> >> The strange thing is: in hosted mode this worked fine, but not in the >> browser. It caused an unhandled exception to be shown (luckily I had a >> handler installed). >> >> I looked at the source of CellPanel and it turns out that the methods >> for setting cell width and height are asymetric to the implementions >> of setting horizontal/vertical alignment. >> >> /** >> * Sets the height of the cell associated with the given widget, >> related to >> * the panel as a whole. >> * >> * @param w the widget whose cell height is to be set >> * @param height the cell's height, in CSS units >> */ >> public void setCellHeight(Widget w, String height) { >> Element td = DOM.getParent(w.getElement()); >> DOM.setElementProperty(td, "height", height); >> } >> >> /** >> * Sets the horizontal alignment of the given widget within its >> cell. >> * >> * @param w the widget whose horizontal alignment is to be set >> * @param align the widget's horizontal alignment, as defined in >> * {...@link HasHorizontalAlignment}. >> */ >> public void setCellHorizontalAlignment(Widget w, >> HorizontalAlignmentConstant align) { >> Element td = getWidgetTd(w); >> if (td != null) { >> setCellHorizontalAlignment(td, align); >> } >> } >> >> as you can see there is a null pointer check here and the getWidgetTd >> method checks if the parent is indeed this CellPanel. >> >> Do I create a bugid ? I could fix it for you if you want ? >> >> David >> >> > > > > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
