Echoing a conversation that should have happened on contrib: jlabanca: This will probably break CellPanel<http://www.google.com/codesearch/p?hl=en#A1edwVHBClQ/user/src/com/google/gwt/user/client/ui/CellPanel.java&q=CellPanel.java%20package:http://google-web-toolkit%5C.googlecode%5C.com&sa=N&cd=1&ct=rc>, and more generally it will break any panel that has a defined inner structure (ie, child widgets are not attached to the outermost DOM element). CellPanel keeps track of its inner tbody element. If you you clear the innerHtml, you'll wipe the tbody element. When you try to add a new element, it will be added to the old tbody reference, which no longer exists. I also have a feeling that IE will spit fireballs if you even try to set innerHTML on a table element.
DecoratorPanel and DialogBox both have defined structures that would be destroyed by the innerHTML call. There may be other panels with a similar problem. rjrjr: John, would it be a reasonable change if CellPanel, DecoratorPanel and DialogBox provided custom overrides for the clear(boolean) method? I suspect the answer is that the answer is "too easy to miss other cases, and too easy to get wrong with the next fancy panel," but I'd like to hear it from you. jlabanca: We could safely hit all of the cases in our code, but it'll be a pain point for subclasses of Panel, especially for external users who already override Panel, which is probably a lot. Panel currently makes no assumptions about layout, so the concrete implementation probably doesn't belong there. I think we should only add it to panels that allow the inner contents to be modified as html, such as HtmlPanel and HtmlTable. Other than that, it seems like it would just defer to clear() anyway. If we have to have a clear() method in PopupPanel, then it should throw an UnsupportedOperationException like Panel#add() does. We can add a package protected utility method to do the default behavior of clearing the innerHtml on the top element. If we do that, I'll vote maybe. On Thu, Jun 17, 2010 at 7:13 AM, <markovuksano...@gmail.com> wrote: > The last patchset seems to be working (both dev and web mode tests have > passed). > > > > > http://gwt-code-reviews.appspot.com/612802/show > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
