Reviewers: jgw, Description: This patch adds an assertion in RootPanel.detachOnWindowClose to check that the wrapped widget's element isn't an ancestor of an already wrapped-widget's element.
Please review this at http://gwt-code-reviews.appspot.com/61813 Affected files: user/src/com/google/gwt/user/client/ui/RootPanel.java user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java Index: user/src/com/google/gwt/user/client/ui/RootPanel.java =================================================================== --- user/src/com/google/gwt/user/client/ui/RootPanel.java (revision 6016) +++ user/src/com/google/gwt/user/client/ui/RootPanel.java (working copy) @@ -121,6 +121,9 @@ + "for the same widget"; assert !isElementChildOfWidget(widget.getElement()) : "A widget that has " + "an existing parent widget may not be added to the detach list"; + assert !isElementAncestorOfWidgetToDetach(widget.getElement()) : "A widget may not " + + "be added to the detach list if it has a child widget that already is " + + "in the detach list"; widgetsToDetach.add(widget); } @@ -256,6 +259,20 @@ } /* + * Checks to see whether the given element is an ancestor of any widget to + * detach. This is not terribly efficient, and is thus only used + * in an assertion. + */ + private static boolean isElementAncestorOfWidgetToDetach(Element element) { + for (Widget widget : widgetsToDetach) { + if (element.isOrHasChild(widget.getElement())) { + return true; + } + } + return false; + } + + /* * Checks to see whether the given element has any parent element that * belongs to a widget. This is not terribly efficient, and is thus only used * in an assertion. Index: user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java =================================================================== --- user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java (revision 6016) +++ user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java (working copy) @@ -314,6 +314,26 @@ } /** + * Tests that wrapping an element that is an ancestor of an existing + * widget's element fails. + */ + public void testWrappingParentElementFails() { + // Testing hosted-mode-only assertion. + if (!GWT.isScript()) { + try { + ensureDiv().setInnerHTML( + "<div id='foo'><a id='bar' href='" + TEST_URL + "'>myAnchor</a></div>"); + Anchor.wrap(Document.get().getElementById("bar")); // pass + + HTML.wrap(Document.get().getElementById("foo")); + fail("Attempting to wrap the above element should have failed."); + } catch (AssertionError e) { + // Expected error. + } + } + } + + /** * Tests that wrap() may only be called on elements that are already attached * to the DOM. */ --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
