From 5ecb7706241683a053c138dfdaa5b5af47297675 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 9 Oct 2024 15:17:46 -0500 Subject: [PATCH] Deprecate and avoid unload event handler (#9984) Event handling for `unload` and `beforeunload` now is split, so that only explicit calls to `WindowImpl.initWindowCloseHandler()`, `Window.addCloseHandler()`, or `Window.addWindowCloseListener()` can cause the `unload` event handler to be attached - all of which are now deprecated. The `beforeunload` event wiring is now the only event handler set up when `Window.addWindowClosingHandler` etc are called. RootPanel methods formerly intended to prevent memory leaks during the `unload` event are deprecated, since modern browsers do not leak these handlers, and may prevent correct behavior of apps using bfcache. Documentation changed on using various static wrap(Element) methods to reflect how to detach, if desired, when the page is closed or no longer in use. Fixes #9908 --- .../com/google/gwt/user/client/Window.java | 18 ++++-- .../gwt/user/client/impl/WindowImpl.java | 39 ++++++++----- .../com/google/gwt/user/client/ui/Anchor.java | 2 +- .../com/google/gwt/user/client/ui/Button.java | 2 +- .../google/gwt/user/client/ui/FileUpload.java | 2 +- .../google/gwt/user/client/ui/FormPanel.java | 4 +- .../com/google/gwt/user/client/ui/Frame.java | 2 +- .../com/google/gwt/user/client/ui/HTML.java | 2 +- .../google/gwt/user/client/ui/HTMLPanel.java | 2 +- .../com/google/gwt/user/client/ui/Hidden.java | 2 +- .../com/google/gwt/user/client/ui/Image.java | 2 +- .../google/gwt/user/client/ui/InlineHTML.java | 2 +- .../gwt/user/client/ui/InlineLabel.java | 2 +- .../com/google/gwt/user/client/ui/Label.java | 2 +- .../google/gwt/user/client/ui/ListBox.java | 2 +- .../gwt/user/client/ui/PasswordTextBox.java | 2 +- .../gwt/user/client/ui/ResetButton.java | 2 +- .../google/gwt/user/client/ui/RootPanel.java | 58 ++++++++++++------- .../gwt/user/client/ui/SimpleCheckBox.java | 2 +- .../gwt/user/client/ui/SimpleRadioButton.java | 2 +- .../gwt/user/client/ui/SubmitButton.java | 2 +- .../google/gwt/user/client/ui/SuggestBox.java | 2 +- .../google/gwt/user/client/ui/TextArea.java | 2 +- .../google/gwt/user/client/ui/TextBox.java | 2 +- .../google/gwt/user/client/ui/ValueBox.java | 2 +- .../google/gwt/user/client/ui/ValueLabel.java | 4 +- .../user/client/ui/ElementWrappingTest.java | 31 ++++++++-- .../google/gwt/user/client/ui/ImageTest.java | 50 ++++++++++++++++ .../gwt/user/client/ui/RootPanelTest.java | 15 +++++ 29 files changed, 188 insertions(+), 73 deletions(-) diff --git a/user/src/com/google/gwt/user/client/Window.java b/user/src/com/google/gwt/user/client/Window.java index 2fb2789f482..972f16d3107 100644 --- a/user/src/com/google/gwt/user/client/Window.java +++ b/user/src/com/google/gwt/user/client/Window.java @@ -506,6 +506,7 @@ public HandlerManager getHandlers() { // Package protected for testing. static WindowHandlers handlers; private static boolean closeHandlersInitialized; + private static boolean beforeCloseHandlersInitialized; private static boolean scrollHandlersInitialized; private static boolean resizeHandlersInitialized; private static int lastResizeWidth; @@ -518,7 +519,10 @@ public HandlerManager getHandlers() { * * @param handler the handler * @return returns the handler registration + * @deprecated This method requires the use of the {@code unload} browser event, which is + * deprecated in all browsers. */ + @Deprecated public static HandlerRegistration addCloseHandler(CloseHandler handler) { maybeInitializeCloseHandlers(); return addHandler(CloseEvent.getType(), handler); @@ -531,7 +535,6 @@ public static HandlerRegistration addCloseHandler(CloseHandler handler) * @return returns the handler registration */ public static HandlerRegistration addResizeHandler(ResizeHandler handler) { - maybeInitializeCloseHandlers(); maybeInitializeResizeHandlers(); return addHandler(ResizeEvent.getType(), handler); } @@ -556,7 +559,7 @@ public static void addWindowCloseListener(WindowCloseListener listener) { */ public static HandlerRegistration addWindowClosingHandler( ClosingHandler handler) { - maybeInitializeCloseHandlers(); + maybeInitializeBeforeCloseHandlers(); return addHandler(Window.ClosingEvent.getType(), handler); } @@ -579,7 +582,6 @@ public static void addWindowResizeListener(WindowResizeListener listener) { */ public static HandlerRegistration addWindowScrollHandler( Window.ScrollHandler handler) { - maybeInitializeCloseHandlers(); maybeInitializeScrollHandlers(); return addHandler(Window.ScrollEvent.getType(), handler); } @@ -910,13 +912,21 @@ private static WindowHandlers getHandlers() { return handlers; } + @Deprecated private static void maybeInitializeCloseHandlers() { if (GWT.isClient() && !closeHandlersInitialized) { - impl.initWindowCloseHandler(); + impl.initWindowUnloadHandler(); closeHandlersInitialized = true; } } + private static void maybeInitializeBeforeCloseHandlers() { + if (GWT.isClient() && !beforeCloseHandlersInitialized) { + impl.initWindowBeforeUnloadHandler(); + beforeCloseHandlersInitialized = true; + } + } + private static void maybeInitializeResizeHandlers() { if (GWT.isClient() && !resizeHandlersInitialized) { impl.initWindowResizeHandler(); diff --git a/user/src/com/google/gwt/user/client/impl/WindowImpl.java b/user/src/com/google/gwt/user/client/impl/WindowImpl.java index 4854d617b83..fc7b78b3361 100644 --- a/user/src/com/google/gwt/user/client/impl/WindowImpl.java +++ b/user/src/com/google/gwt/user/client/impl/WindowImpl.java @@ -29,11 +29,30 @@ public native String getHash() /*-{ public native String getQueryString() /*-{ return $wnd.location.search; }-*/; - - public native void initWindowCloseHandler() /*-{ + + @Deprecated + public void initWindowCloseHandler() { + initWindowUnloadHandler(); + initWindowBeforeUnloadHandler(); + } + + @Deprecated + public native void initWindowUnloadHandler() /*-{ + var oldOnUnload = $wnd.onunload; + + $wnd.onunload = $entry(function(evt) { + try { + @com.google.gwt.user.client.Window::onClosed()(); + } finally { + oldOnUnload && oldOnUnload(evt); + $wnd.onunload = null; + } + }); + }-*/; + + public native void initWindowBeforeUnloadHandler() /*-{ var oldOnBeforeUnload = $wnd.onbeforeunload; - var oldOnUnload = $wnd.onunload; - + // Old mozilla doesn't like $entry's explicit return statement and // will always pop up a confirmation dialog. This is worked around by // just wrapping the call to onClosing(), which still has the semantics @@ -55,18 +74,6 @@ public native void initWindowCloseHandler() /*-{ } // returns undefined. }; - - $wnd.onunload = $entry(function(evt) { - try { - @com.google.gwt.user.client.Window::onClosed()(); - } finally { - oldOnUnload && oldOnUnload(evt); - $wnd.onresize = null; - $wnd.onscroll = null; - $wnd.onbeforeunload = null; - $wnd.onunload = null; - } - }); }-*/; public native void initWindowResizeHandler() /*-{ diff --git a/user/src/com/google/gwt/user/client/ui/Anchor.java b/user/src/com/google/gwt/user/client/ui/Anchor.java index 02ecaef6761..77d4a8b37f1 100644 --- a/user/src/com/google/gwt/user/client/ui/Anchor.java +++ b/user/src/com/google/gwt/user/client/ui/Anchor.java @@ -73,7 +73,7 @@ public class Anchor extends FocusWidget implements HasHorizontalAlignment, * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/Button.java b/user/src/com/google/gwt/user/client/ui/Button.java index b1e5aec0585..d31f6e17869 100644 --- a/user/src/com/google/gwt/user/client/ui/Button.java +++ b/user/src/com/google/gwt/user/client/ui/Button.java @@ -47,7 +47,7 @@ public class Button extends ButtonBase { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/FileUpload.java b/user/src/com/google/gwt/user/client/ui/FileUpload.java index 228fae0a4dd..53b205e620d 100644 --- a/user/src/com/google/gwt/user/client/ui/FileUpload.java +++ b/user/src/com/google/gwt/user/client/ui/FileUpload.java @@ -54,7 +54,7 @@ public class FileUpload extends FocusWidget implements HasName, HasChangeHandler * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/FormPanel.java b/user/src/com/google/gwt/user/client/ui/FormPanel.java index 396f8d609bf..a64f85e3315 100644 --- a/user/src/com/google/gwt/user/client/ui/FormPanel.java +++ b/user/src/com/google/gwt/user/client/ui/FormPanel.java @@ -253,7 +253,7 @@ interface IFrameTemplate extends SafeHtmlTemplates { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * *

* The specified form element's target attribute will not be set, and the @@ -280,7 +280,7 @@ public static FormPanel wrap(Element element) { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * *

* If the createIFrame parameter is set to true, then the wrapped diff --git a/user/src/com/google/gwt/user/client/ui/Frame.java b/user/src/com/google/gwt/user/client/ui/Frame.java index 1d0f7bca9bd..4cdf92eb9a8 100644 --- a/user/src/com/google/gwt/user/client/ui/Frame.java +++ b/user/src/com/google/gwt/user/client/ui/Frame.java @@ -52,7 +52,7 @@ public class Frame extends Widget implements HasLoadHandlers { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/HTML.java b/user/src/com/google/gwt/user/client/ui/HTML.java index 25dc24a6954..b6704a35201 100644 --- a/user/src/com/google/gwt/user/client/ui/HTML.java +++ b/user/src/com/google/gwt/user/client/ui/HTML.java @@ -61,7 +61,7 @@ public class HTML extends Label * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/HTMLPanel.java b/user/src/com/google/gwt/user/client/ui/HTMLPanel.java index 580f0815a1c..926424e7416 100644 --- a/user/src/com/google/gwt/user/client/ui/HTMLPanel.java +++ b/user/src/com/google/gwt/user/client/ui/HTMLPanel.java @@ -50,7 +50,7 @@ public static String createUniqueId() { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/Hidden.java b/user/src/com/google/gwt/user/client/ui/Hidden.java index d9f1466f8fa..e8807f49d77 100644 --- a/user/src/com/google/gwt/user/client/ui/Hidden.java +++ b/user/src/com/google/gwt/user/client/ui/Hidden.java @@ -34,7 +34,7 @@ public class Hidden extends Widget implements HasName, TakesValue, IsEdi * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/Image.java b/user/src/com/google/gwt/user/client/ui/Image.java index 2c7587941a9..c00cfecda02 100644 --- a/user/src/com/google/gwt/user/client/ui/Image.java +++ b/user/src/com/google/gwt/user/client/ui/Image.java @@ -484,7 +484,7 @@ public static void prefetch(SafeUri url) { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/InlineHTML.java b/user/src/com/google/gwt/user/client/ui/InlineHTML.java index 625f274d47b..9b0cf234e52 100644 --- a/user/src/com/google/gwt/user/client/ui/InlineHTML.java +++ b/user/src/com/google/gwt/user/client/ui/InlineHTML.java @@ -55,7 +55,7 @@ public class InlineHTML extends HTML { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/InlineLabel.java b/user/src/com/google/gwt/user/client/ui/InlineLabel.java index d84123630e1..f1ac5544727 100644 --- a/user/src/com/google/gwt/user/client/ui/InlineLabel.java +++ b/user/src/com/google/gwt/user/client/ui/InlineLabel.java @@ -46,7 +46,7 @@ public class InlineLabel extends Label { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/Label.java b/user/src/com/google/gwt/user/client/ui/Label.java index a5910a64671..3759a39382c 100644 --- a/user/src/com/google/gwt/user/client/ui/Label.java +++ b/user/src/com/google/gwt/user/client/ui/Label.java @@ -115,7 +115,7 @@ public class Label extends LabelBase implements HasDirectionalText, * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/ListBox.java b/user/src/com/google/gwt/user/client/ui/ListBox.java index fb9f97861c8..78cfd105f2a 100644 --- a/user/src/com/google/gwt/user/client/ui/ListBox.java +++ b/user/src/com/google/gwt/user/client/ui/ListBox.java @@ -99,7 +99,7 @@ public class ListBox extends FocusWidget implements SourcesChangeEvents, * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped * @return list box diff --git a/user/src/com/google/gwt/user/client/ui/PasswordTextBox.java b/user/src/com/google/gwt/user/client/ui/PasswordTextBox.java index 646e44e63df..d1060a33eea 100644 --- a/user/src/com/google/gwt/user/client/ui/PasswordTextBox.java +++ b/user/src/com/google/gwt/user/client/ui/PasswordTextBox.java @@ -46,7 +46,7 @@ public class PasswordTextBox extends TextBox { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/ResetButton.java b/user/src/com/google/gwt/user/client/ui/ResetButton.java index 59ef767d406..75c5a99fe78 100644 --- a/user/src/com/google/gwt/user/client/ui/ResetButton.java +++ b/user/src/com/google/gwt/user/client/ui/ResetButton.java @@ -38,7 +38,7 @@ public class ResetButton extends Button { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/RootPanel.java b/user/src/com/google/gwt/user/client/ui/RootPanel.java index cc571e07c61..7397544b394 100644 --- a/user/src/com/google/gwt/user/client/ui/RootPanel.java +++ b/user/src/com/google/gwt/user/client/ui/RootPanel.java @@ -18,13 +18,10 @@ import com.google.gwt.dom.client.BodyElement; import com.google.gwt.dom.client.Document; import com.google.gwt.dom.client.Element; -import com.google.gwt.event.logical.shared.CloseEvent; -import com.google.gwt.event.logical.shared.CloseHandler; import com.google.gwt.i18n.client.BidiUtils; import com.google.gwt.i18n.client.HasDirection; import com.google.gwt.i18n.client.LocaleInfo; import com.google.gwt.user.client.Event; -import com.google.gwt.user.client.Window; import java.util.HashMap; import java.util.HashSet; @@ -92,11 +89,13 @@ public void execute(Widget w) { * This method may only be called per widget, and only for widgets that were * originally passed to {@link #detachOnWindowClose(Widget)}. *

- * + * * @param widget the widget that no longer needs to be cleaned up when the * page closes * @see #detachOnWindowClose(Widget) + * @deprecated Instead, use {@link Widget#removeFromParent()}. */ + @Deprecated public static void detachNow(Widget widget) { assert widgetsToDetach.contains(widget) : "detachNow() called on a widget " + "not currently in the detach list"; @@ -111,25 +110,30 @@ public static void detachNow(Widget widget) { /** * Adds a widget to the detach list. This is the list of widgets to be * detached when the page unloads. - * + * *

* This method must be called for all widgets that have no parent widgets. * These are most commonly {@link RootPanel RootPanels}, but can also be any * widget used to wrap an existing element on the page. Failing to do this may * cause these widgets to leak memory. This method is called automatically by * widgets' wrap methods (e.g. - * {@link Button#wrap(com.google.gwt.dom.client.Element)}). + * {@link Button#wrap(Element)}). *

- * + * *

* This method may not be called on any widget whose element is * contained in another widget. This is to ensure that the DOM and Widget * hierarchies cannot get into an inconsistent state. *

- * + * * @param widget the widget to be cleaned up when the page closes * @see #detachNow(Widget) + * @deprecated While originally introduced to combat memory leaks in old browsers, this is no + * longer necessary, and the unload event used by this method is being removed from browsers. + * Additionally, it is unreliable as a means to ensure calls to {@link Widget#onUnload()}. See + * Issue 9908 for more information. */ + @Deprecated public static void detachOnWindowClose(Widget widget) { assert !widgetsToDetach.contains(widget) : "detachOnUnload() called twice " + "for the same widget"; @@ -188,8 +192,6 @@ public static RootPanel get(String id) { // on the first RootPanel.get(String) or RootPanel.get() // call. if (rootPanels.size() == 0) { - hookWindowClosing(); - // If we're in a RTL locale, set the RTL directionality // on the entire document. if (LocaleInfo.getCurrentLocale().isRTL()) { @@ -223,16 +225,37 @@ public static native com.google.gwt.user.client.Element getBodyElement() /*-{ /** * Determines whether the given widget is in the detach list. - * + *

+ * Note that modern browsers do not have the memory leaks that originally required use of this + * feature - it is retained only to support application-specific detach events. + *

+ * * @param widget the widget to be checked * @return true if the widget is in the detach list + * @deprecated Use of the detach list requires the unload event, which is planned to be removed + * in future browser updates. See notice on {@link #detachOnWindowClose(Widget)} and + * Issue 9908 for more information. */ + @Deprecated public static boolean isInDetachList(Widget widget) { return widgetsToDetach.contains(widget); } - // Package-protected for use by unit tests. Do not call this method directly. - static void detachWidgets() { + /** + * Detaches all widgets that were set to be detached on window close by a call to + * {@link #detachOnWindowClose}. Formerly was package-protected, now can be called by projects + * that required the old behavior and are willing to set up their own onclose handler on the + * window. + *

+ * Note that modern browsers do not have the memory leaks that originally required use of this + * feature - it is retained only to support application-specific detach events. + *

+ * + * @deprecated See notice on {@link #detachOnWindowClose(Widget)} and + * Issue 9908 for more information. + */ + @Deprecated + public static void detachWidgets() { // When the window is closing, detach all widgets that need to be // cleaned up. This will cause all of their event listeners // to be unhooked, which will avoid potential memory leaks. @@ -258,15 +281,6 @@ private static native Element getRootElement() /*-{ return $doc; }-*/; - private static void hookWindowClosing() { - // Catch the window closing event. - Window.addCloseHandler(new CloseHandler() { - public void onClose(CloseEvent closeEvent) { - detachWidgets(); - } - }); - } - /* * 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 diff --git a/user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java b/user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java index 2e493ba2f3f..0e71670717a 100644 --- a/user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java +++ b/user/src/com/google/gwt/user/client/ui/SimpleCheckBox.java @@ -45,7 +45,7 @@ public class SimpleCheckBox extends FocusWidget implements HasName, * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/SimpleRadioButton.java b/user/src/com/google/gwt/user/client/ui/SimpleRadioButton.java index 5d415fef058..4a7b8982420 100644 --- a/user/src/com/google/gwt/user/client/ui/SimpleRadioButton.java +++ b/user/src/com/google/gwt/user/client/ui/SimpleRadioButton.java @@ -36,7 +36,7 @@ public class SimpleRadioButton extends SimpleCheckBox { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/SubmitButton.java b/user/src/com/google/gwt/user/client/ui/SubmitButton.java index 82a770e358e..199c3808eb2 100644 --- a/user/src/com/google/gwt/user/client/ui/SubmitButton.java +++ b/user/src/com/google/gwt/user/client/ui/SubmitButton.java @@ -39,7 +39,7 @@ public class SubmitButton extends Button { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/SuggestBox.java b/user/src/com/google/gwt/user/client/ui/SuggestBox.java index c366ab79378..55fa3383290 100644 --- a/user/src/com/google/gwt/user/client/ui/SuggestBox.java +++ b/user/src/com/google/gwt/user/client/ui/SuggestBox.java @@ -635,7 +635,7 @@ public void setSuggestion(Suggestion suggestion) { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param oracle the suggest box oracle to use * @param element the element to be wrapped diff --git a/user/src/com/google/gwt/user/client/ui/TextArea.java b/user/src/com/google/gwt/user/client/ui/TextArea.java index 0efdb67e7dc..e2f9e404502 100644 --- a/user/src/com/google/gwt/user/client/ui/TextArea.java +++ b/user/src/com/google/gwt/user/client/ui/TextArea.java @@ -52,7 +52,7 @@ public class TextArea extends TextBoxBase { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/TextBox.java b/user/src/com/google/gwt/user/client/ui/TextBox.java index 0143c88666b..97d2dcbabf9 100644 --- a/user/src/com/google/gwt/user/client/ui/TextBox.java +++ b/user/src/com/google/gwt/user/client/ui/TextBox.java @@ -54,7 +54,7 @@ public class TextBox extends TextBoxBase { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/ValueBox.java b/user/src/com/google/gwt/user/client/ui/ValueBox.java index 33b75e86017..08e1741858f 100644 --- a/user/src/com/google/gwt/user/client/ui/ValueBox.java +++ b/user/src/com/google/gwt/user/client/ui/ValueBox.java @@ -35,7 +35,7 @@ public class ValueBox extends ValueBoxBase { * * This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped */ diff --git a/user/src/com/google/gwt/user/client/ui/ValueLabel.java b/user/src/com/google/gwt/user/client/ui/ValueLabel.java index 9607da3c0d3..d5f4fe56f08 100644 --- a/user/src/com/google/gwt/user/client/ui/ValueLabel.java +++ b/user/src/com/google/gwt/user/client/ui/ValueLabel.java @@ -45,7 +45,7 @@ public class ValueLabel extends LabelBase implements TakesValue, *

* This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped * @param renderer the renderer used to render values into the element @@ -72,7 +72,7 @@ public static ValueLabel wrap(Element element, *

* This element must already be attached to the document. If the element is * removed from the document, you must call - * {@link RootPanel#detachNow(Widget)}. + * {@link Widget#removeFromParent()}. * * @param element the element to be wrapped * @param renderer the renderer used to render values into the element diff --git a/user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java b/user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java index 14eba2c5fef..6bb932e246e 100644 --- a/user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java +++ b/user/test/com/google/gwt/user/client/ui/ElementWrappingTest.java @@ -64,13 +64,13 @@ public void testButton() { public void testDetachNowTwiceFails() { // Testing hosted-mode-only assertion. if (!GWT.isScript()) { + // Trying to pass the same widget to RootPanel.detachNow() twice + // should fail an assertion. + ensureDiv().setInnerHTML( + "myAnchor"); + Anchor a = Anchor.wrap(Document.get().getElementById("foo")); + RootPanel.detachNow(a); // pass try { - // Trying to pass the same widget to RootPanel.detachNow() twice - // should fail an assertion. - ensureDiv().setInnerHTML( - "myAnchor"); - Anchor a = Anchor.wrap(Document.get().getElementById("foo")); - RootPanel.detachNow(a); // pass RootPanel.detachNow(a); // fail throw new Error("Expected assertion failure calling detachNow() twice"); } catch (AssertionError e) { @@ -78,6 +78,25 @@ public void testDetachNowTwiceFails() { } } + /** + * Tests that {@link Widget#removeFromParent()} can be called more than once and successfully + * detaches wrapped widgets. + */ + public void testRemoveFromParentDuplicateSucceeds() { + // Testing hosted-mode-only assertion. + if (!GWT.isScript()) { + // Trying to pass the same widget to RootPanel.detachNow() twice + // should fail an assertion. + ensureDiv().setInnerHTML( + "myAnchor"); + Anchor a = Anchor.wrap(Document.get().getElementById("foo")); + a.removeFromParent(); // success + assertFalse(RootPanel.isInDetachList(a)); + a.removeFromParent(); // no-op + assertFalse(RootPanel.isInDetachList(a)); + } + } + /** * Tests that {@link RootPanel#detachOnWindowClose(Widget)} can only be called * once per widget. diff --git a/user/test/com/google/gwt/user/client/ui/ImageTest.java b/user/test/com/google/gwt/user/client/ui/ImageTest.java index 6ee5dd39efb..4123ff715d1 100644 --- a/user/test/com/google/gwt/user/client/ui/ImageTest.java +++ b/user/test/com/google/gwt/user/client/ui/ImageTest.java @@ -790,10 +790,60 @@ public void testWrapOfSubclass() { assertNotNull(image); // Cleanup. + Document.get().getBody().removeChild(div); + RootPanel.detachNow(image); + } + + /** + * Same test, but don't remove from the dom first + */ + public void testWrapOfSubclassWithoutRemove() { + String uid = Document.get().createUniqueId(); + DivElement div = Document.get().createDivElement(); + div.setInnerHTML(""); Document.get().getBody().appendChild(div); + + final TestImage image = TestImage.wrap(Document.get().getElementById(uid)); + assertNotNull(image); + + // Cleanup. RootPanel.detachNow(image); } + /** + * Same test, but with removeFromParent instead of detachNow + */ + public void testRemoveFromParent() { + String uid = Document.get().createUniqueId(); + DivElement div = Document.get().createDivElement(); + div.setInnerHTML(""); + Document.get().getBody().appendChild(div); + + final TestImage image = TestImage.wrap(Document.get().getElementById(uid)); + assertNotNull(image); + + // Cleanup. + Document.get().getBody().removeChild(div); + image.removeFromParent(); + } + + /** + * Same test, but with removeFromParent isntead of detachNow, and don't remove the dom manually + * first. + */ + public void testRemoveFromParentWithoutRemove() { + String uid = Document.get().createUniqueId(); + DivElement div = Document.get().createDivElement(); + div.setInnerHTML(""); + Document.get().getBody().appendChild(div); + + final TestImage image = TestImage.wrap(Document.get().getElementById(uid)); + assertNotNull(image); + + // Cleanup. + image.removeFromParent(); + } + /** * Tests that wrapping an existing DOM element works if you call * setUrlAndVisibleRect() on it. diff --git a/user/test/com/google/gwt/user/client/ui/RootPanelTest.java b/user/test/com/google/gwt/user/client/ui/RootPanelTest.java index d4a77a72be6..2d8fff4b3fc 100644 --- a/user/test/com/google/gwt/user/client/ui/RootPanelTest.java +++ b/user/test/com/google/gwt/user/client/ui/RootPanelTest.java @@ -96,6 +96,21 @@ public void testDetachNowWithErrorOnDetach() { assertFalse(RootPanel.isInDetachList(w)); } + public void testRemoveFromParentWithErrorOnDetach() { + BadWidget w = BadWidget.wrap(createAttachedDivElement()); + w.setFailOnUnload(true); + assertTrue(RootPanel.isInDetachList(w)); + assertTrue(RootPanel.getBodyElement().isOrHasChild(w.getElement())); + + try { + w.removeFromParent(); + fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // Expected. + } + assertFalse(RootPanel.isInDetachList(w)); + } + public void testDetachWidgetsWithErrorOnDetach() { BadWidget bad0 = BadWidget.wrap(createAttachedDivElement()); bad0.setFailOnUnload(true);