Oh cool, Stephen provided the patch for the robust fix.

On Wed, Oct 13, 2010 at 3:53 PM, Ray Ryan <[email protected]> wrote:

> These events are provided so that you can accomplish the same kinds of
> tasks without having to subclass the Widget, so it seemed natural (well,
> easier) to put the behavior there. But maybe that is too fragile, I'll try
> to move it where it belongs.
>
> Another question is why Panel overrides those methods in the first place.
> I'll delete those, they're noise.
>
> Stephen, thanks for catching this, and especially for punching up the test.
>
>
> On Tue, Oct 12, 2010 at 12:37 PM, Joel Webber <[email protected]> wrote:
>
>> Hmm... I didn't even realize AttachEvent had been added. John & Ray, you
>> might want to take a look at this -- in the original design this super
>> invocation wouldn't have mattered, because widget's onLoad() was empty
>> (onAttach/Detach() weren't really meant to overridden outside of Panel and
>> Composite -- onLoad/Unload() were the methods to override). But now it
>> appears that Widget.onLoad() has actual code in it. Could this not be moved
>> to onAttach?
>>
>> Le 11 octobre 2010 14:05, <[email protected]> a écrit :
>>
>>  Reviewers: ,
>>>
>>> Description:
>>> Panel's onLoad does not call super.onLoad. This is a simple fix that
>>> just adds the missing onLoad call.
>>>
>>> I have another patch set which fires the attach event in
>>> onAttach/onDetach, given these are more stable methods that user's are
>>> less prone overriding them.
>>>
>>> Please review this at http://gwt-code-reviews.appspot.com/981801/show
>>>
>>> Affected files:
>>>  user/src/com/google/gwt/user/client/ui/Panel.java
>>>  user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java
>>>
>>>
>>> Index: user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java
>>> ===================================================================
>>> --- user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java
>>> (revision 8929)
>>> +++ user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java
>>> (working copy)
>>> @@ -121,9 +121,12 @@
>>>     }
>>>
>>>     TestPanel tp = new TestPanel();
>>> +    TestAttachHandler tpa = new TestAttachHandler();
>>> +    tp.addAttachHandler(tpa);
>>> +
>>>     TestWidget tw = new TestWidget();
>>> -    TestAttachHandler ta = new TestAttachHandler();
>>> -    tw.addAttachHandler(ta);
>>> +    TestAttachHandler twa = new TestAttachHandler();
>>> +    tw.addAttachHandler(twa);
>>>
>>>     tp.add(tw);
>>>     RootPanel.get().add(tp);
>>> @@ -134,9 +137,9 @@
>>>     assertTrue(tp.onAttachOrder < tp.onLoadOrder);
>>>     assertTrue(tp.onDetachOrder < tp.onUnloadOrder);
>>>     assertTrue(tw.onAttachOrder < tw.onLoadOrder);
>>> -    assertTrue(tw.onLoadOrder < ta.delegateAttachOrder);
>>> +    assertTrue(tw.onLoadOrder < twa.delegateAttachOrder);
>>>     assertTrue(tw.onDetachOrder < tw.onUnloadOrder);
>>> -    assertTrue(tw.onUnloadOrder < ta.delegateDetachOrder);
>>> +    assertTrue(tw.onUnloadOrder < twa.delegateDetachOrder);
>>>
>>>     // Also trivial. Ensure that the panel's onAttach/onDetach is called
>>> before
>>>     // its child's onAttach/onDetach.
>>> @@ -150,6 +153,9 @@
>>>     // detached/unloaded.
>>>     assertTrue(tp.onUnloadOrder < tw.onUnloadOrder);
>>>
>>> +    assertTrue(tp.onLoadOrder < tpa.delegateAttachOrder);
>>> +    assertTrue(tp.onUnloadOrder < tpa.delegateDetachOrder);
>>> +
>>>     // Make sure each widget/panel's elements are actually still attached
>>> to the
>>>     // DOM during onLoad/onUnload.
>>>     assertTrue(tp.domAttachedOnLoad);
>>> Index: user/src/com/google/gwt/user/client/ui/Panel.java
>>> ===================================================================
>>> --- user/src/com/google/gwt/user/client/ui/Panel.java   (revision 8929)
>>> +++ user/src/com/google/gwt/user/client/ui/Panel.java   (working copy)
>>> @@ -183,6 +183,7 @@
>>>    */
>>>   @Override
>>>   protected void onLoad() {
>>> +    super.onLoad();
>>>   }
>>>
>>>   /**
>>> @@ -193,6 +194,7 @@
>>>    */
>>>   @Override
>>>   protected void onUnload() {
>>> +    super.onUnload();
>>>   }
>>>
>>>   /**
>>>
>>>
>>> --
>>> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>>>
>>
>>
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to