You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Sven Meier <sv...@meiers.net> on 2013/01/17 11:17:22 UTC

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

>	@Override
>	public void detach(Component component)
>	{
>		AjaxRequestTarget target = component.getRequestCycle().find(AjaxRequestTarget.class);
>		if (target != null)
>		{
>			stop(target);
>		}
>		super.detach(component);
>	}


A small nitpick:
Removing a component in an ART might not be the only reason why 
#detach() is called. A developer might call component#detach() to force 
a LDM to reload on the next render.

Sven

On 01/17/2013 10:20 AM, mgrigorov@apache.org wrote:
> WICKET-4959 Unproperly detached Behavior with TabbedPanels
>
> Stop the Ajax timer behavior when the behavior's component is removed from the tree
>
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/0b78d759
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/0b78d759
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/0b78d759
>
> Branch: refs/heads/master
> Commit: 0b78d759220c1b09abb0d47b5007757bbfeb4e0c
> Parents: e37a9e1
> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
> Authored: Thu Jan 17 11:18:22 2013 +0200
> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
> Committed: Thu Jan 17 11:18:22 2013 +0200
>
> ----------------------------------------------------------------------
>   .../wicket/ajax/AbstractAjaxTimerBehavior.java     |   11 +++++++++++
>   1 files changed, 11 insertions(+), 0 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/0b78d759/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
> ----------------------------------------------------------------------
> diff --git a/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java b/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
> index 83edeaa..a80921d 100644
> --- a/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
> +++ b/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
> @@ -194,4 +194,15 @@ public abstract class AbstractAjaxTimerBehavior extends AbstractDefaultAjaxBehav
>   		String timeoutHandle = getTimeoutHandle();
>   		target.prependJavaScript("clearTimeout("+timeoutHandle+"); delete "+timeoutHandle+";");
>   	}
> +
> +	@Override
> +	public void detach(Component component)
> +	{
> +		AjaxRequestTarget target = component.getRequestCycle().find(AjaxRequestTarget.class);
> +		if (target != null)
> +		{
> +			stop(target);
> +		}
> +		super.detach(component);
> +	}
>   }
>


Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Martin Grigorov <mg...@apache.org>.
On Thu, Jan 17, 2013 at 12:53 PM, Ernesto Reinaldo Barreiro <
reiern70@gmail.com> wrote:

> Hi,
>
> I see there is
>
> /**
>  * Remove any scheduled timers on the removed element.
>  * This wont remove the timer for elements which are children of the
> removed one.
>  */
> Wicket.Event.subscribe('/dom/node/removing', function(jqEvent, element) {
> var id = element.id;
> if (Wicket.TimerHandles && Wicket.TimerHandles[id]) {
> window.clearTimeout(Wicket.TimerHandles[id]);
> delete Wicket.TimerHandles[id];
> }
> });
>
> would this take care of automatically removing the behavior at client side?
>

Only if the html element with the registered timer behavior is
removed/replaced.
If a parent element is removed/replaced then this wont be able to clean up.


>
> On Thu, Jan 17, 2013 at 11:49 AM, Ernesto Reinaldo Barreiro <
> reiern70@gmail.com> wrote:
>
> > Or maybe "mark" those behaviors as "if it fails do not throw exception".
> > That way when call back comes behavior will be removed form client side
> and
> > failing request ignored?
> >
> >
> > On Thu, Jan 17, 2013 at 11:44 AM, Sven Meier <sv...@meiers.net> wrote:
> >
> >> Yeah, this is what I wrote in my first comment:
> >>
> >> "Thus the next pending Ajax request comes through, since wicket-ajax's
> >> defaultPrecondition sees the markupId still present in the dom."
> >>
> >> Perhaps we have to cancel out all scheduled callbacks for components
> >> which are replaced via Ajax.
> >>
> >> Sven
> >>
> >>
> >> On 01/17/2013 11:34 AM, Ernesto Reinaldo Barreiro wrote:
> >>
> >>> Hi,
> >>>
> >>> On Thu, Jan 17, 2013 at 11:17 AM, Sven Meier <sv...@meiers.net> wrote:
> >>>
> >>>           @Override
> >>>>
> >>>>>          public void detach(Component component)
> >>>>>          {
> >>>>>                  AjaxRequestTarget target =
> >>>>> component.getRequestCycle().**
> >>>>>
> >>>>> find(AjaxRequestTarget.class);
> >>>>>                  if (target != null)
> >>>>>                  {
> >>>>>                          stop(target);
> >>>>>                  }
> >>>>>                  super.detach(component);
> >>>>>          }
> >>>>>
> >>>>>
> >>>> A small nitpick:
> >>>> Removing a component in an ART might not be the only reason why
> >>>> #detach()
> >>>> is called. A developer might call component#detach() to force a LDM to
> >>>> reload on the next render.
> >>>>
> >>>> Sven
> >>>>
> >>>>
> >>>>  Just another caveat: what happens if during rending time, before call
> >>> AJAX
> >>> response is processed, the behavior is re-fired at client side?
> Wouldn't
> >>>   this produce a component not found exception?
> >>>
> >>>
> >>
> >
> >
> > --
> > Regards - Ernesto Reinaldo Barreiro
> > Antilia Soft
> > http://antiliasoft.com/ <http://antiliasoft.com/antilia>
> >
>
>
>
> --
> Regards - Ernesto Reinaldo Barreiro
> Antilia Soft
> http://antiliasoft.com/ <http://antiliasoft.com/antilia>
>



-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
ok. forget this comment. JavaScript doc is very explicit.

On Thu, Jan 17, 2013 at 11:53 AM, Ernesto Reinaldo Barreiro <
reiern70@gmail.com> wrote:

> Hi,
>
> I see there is
>
> /**
>  * Remove any scheduled timers on the removed element.
>  * This wont remove the timer for elements which are children of the
> removed one.
>  */
> Wicket.Event.subscribe('/dom/node/removing', function(jqEvent, element) {
>  var id = element.id;
> if (Wicket.TimerHandles && Wicket.TimerHandles[id]) {
>  window.clearTimeout(Wicket.TimerHandles[id]);
> delete Wicket.TimerHandles[id];
>  }
> });
>
> would this take care of automatically removing the behavior at client side?
>
> On Thu, Jan 17, 2013 at 11:49 AM, Ernesto Reinaldo Barreiro <
> reiern70@gmail.com> wrote:
>
>> Or maybe "mark" those behaviors as "if it fails do not throw exception".
>> That way when call back comes behavior will be removed form client side and
>> failing request ignored?
>>
>>
>>  On Thu, Jan 17, 2013 at 11:44 AM, Sven Meier <sv...@meiers.net> wrote:
>>
>>> Yeah, this is what I wrote in my first comment:
>>>
>>> "Thus the next pending Ajax request comes through, since wicket-ajax's
>>> defaultPrecondition sees the markupId still present in the dom."
>>>
>>> Perhaps we have to cancel out all scheduled callbacks for components
>>> which are replaced via Ajax.
>>>
>>> Sven
>>>
>>>
>>> On 01/17/2013 11:34 AM, Ernesto Reinaldo Barreiro wrote:
>>>
>>>> Hi,
>>>>
>>>> On Thu, Jan 17, 2013 at 11:17 AM, Sven Meier <sv...@meiers.net> wrote:
>>>>
>>>>           @Override
>>>>>
>>>>>>          public void detach(Component component)
>>>>>>          {
>>>>>>                  AjaxRequestTarget target =
>>>>>> component.getRequestCycle().**
>>>>>>
>>>>>> find(AjaxRequestTarget.class);
>>>>>>                  if (target != null)
>>>>>>                  {
>>>>>>                          stop(target);
>>>>>>                  }
>>>>>>                  super.detach(component);
>>>>>>          }
>>>>>>
>>>>>>
>>>>> A small nitpick:
>>>>> Removing a component in an ART might not be the only reason why
>>>>> #detach()
>>>>> is called. A developer might call component#detach() to force a LDM to
>>>>> reload on the next render.
>>>>>
>>>>> Sven
>>>>>
>>>>>
>>>>>  Just another caveat: what happens if during rending time, before call
>>>> AJAX
>>>> response is processed, the behavior is re-fired at client side? Wouldn't
>>>>   this produce a component not found exception?
>>>>
>>>>
>>>
>>
>>
>> --
>> Regards - Ernesto Reinaldo Barreiro
>> Antilia Soft
>> http://antiliasoft.com/ <http://antiliasoft.com/antilia>
>>
>
>
>
> --
> Regards - Ernesto Reinaldo Barreiro
> Antilia Soft
> http://antiliasoft.com/ <http://antiliasoft.com/antilia>
>



-- 
Regards - Ernesto Reinaldo Barreiro
Antilia Soft
http://antiliasoft.com/ <http://antiliasoft.com/antilia>

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
Hi,

I see there is

/**
 * Remove any scheduled timers on the removed element.
 * This wont remove the timer for elements which are children of the
removed one.
 */
Wicket.Event.subscribe('/dom/node/removing', function(jqEvent, element) {
var id = element.id;
if (Wicket.TimerHandles && Wicket.TimerHandles[id]) {
window.clearTimeout(Wicket.TimerHandles[id]);
delete Wicket.TimerHandles[id];
}
});

would this take care of automatically removing the behavior at client side?

On Thu, Jan 17, 2013 at 11:49 AM, Ernesto Reinaldo Barreiro <
reiern70@gmail.com> wrote:

> Or maybe "mark" those behaviors as "if it fails do not throw exception".
> That way when call back comes behavior will be removed form client side and
> failing request ignored?
>
>
> On Thu, Jan 17, 2013 at 11:44 AM, Sven Meier <sv...@meiers.net> wrote:
>
>> Yeah, this is what I wrote in my first comment:
>>
>> "Thus the next pending Ajax request comes through, since wicket-ajax's
>> defaultPrecondition sees the markupId still present in the dom."
>>
>> Perhaps we have to cancel out all scheduled callbacks for components
>> which are replaced via Ajax.
>>
>> Sven
>>
>>
>> On 01/17/2013 11:34 AM, Ernesto Reinaldo Barreiro wrote:
>>
>>> Hi,
>>>
>>> On Thu, Jan 17, 2013 at 11:17 AM, Sven Meier <sv...@meiers.net> wrote:
>>>
>>>           @Override
>>>>
>>>>>          public void detach(Component component)
>>>>>          {
>>>>>                  AjaxRequestTarget target =
>>>>> component.getRequestCycle().**
>>>>>
>>>>> find(AjaxRequestTarget.class);
>>>>>                  if (target != null)
>>>>>                  {
>>>>>                          stop(target);
>>>>>                  }
>>>>>                  super.detach(component);
>>>>>          }
>>>>>
>>>>>
>>>> A small nitpick:
>>>> Removing a component in an ART might not be the only reason why
>>>> #detach()
>>>> is called. A developer might call component#detach() to force a LDM to
>>>> reload on the next render.
>>>>
>>>> Sven
>>>>
>>>>
>>>>  Just another caveat: what happens if during rending time, before call
>>> AJAX
>>> response is processed, the behavior is re-fired at client side? Wouldn't
>>>   this produce a component not found exception?
>>>
>>>
>>
>
>
> --
> Regards - Ernesto Reinaldo Barreiro
> Antilia Soft
> http://antiliasoft.com/ <http://antiliasoft.com/antilia>
>



-- 
Regards - Ernesto Reinaldo Barreiro
Antilia Soft
http://antiliasoft.com/ <http://antiliasoft.com/antilia>

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
Or maybe "mark" those behaviors as "if it fails do not throw exception".
That way when call back comes behavior will be removed form client side and
failing request ignored?

On Thu, Jan 17, 2013 at 11:44 AM, Sven Meier <sv...@meiers.net> wrote:

> Yeah, this is what I wrote in my first comment:
>
> "Thus the next pending Ajax request comes through, since wicket-ajax's
> defaultPrecondition sees the markupId still present in the dom."
>
> Perhaps we have to cancel out all scheduled callbacks for components which
> are replaced via Ajax.
>
> Sven
>
>
> On 01/17/2013 11:34 AM, Ernesto Reinaldo Barreiro wrote:
>
>> Hi,
>>
>> On Thu, Jan 17, 2013 at 11:17 AM, Sven Meier <sv...@meiers.net> wrote:
>>
>>           @Override
>>>
>>>>          public void detach(Component component)
>>>>          {
>>>>                  AjaxRequestTarget target =
>>>> component.getRequestCycle().**
>>>>
>>>> find(AjaxRequestTarget.class);
>>>>                  if (target != null)
>>>>                  {
>>>>                          stop(target);
>>>>                  }
>>>>                  super.detach(component);
>>>>          }
>>>>
>>>>
>>> A small nitpick:
>>> Removing a component in an ART might not be the only reason why #detach()
>>> is called. A developer might call component#detach() to force a LDM to
>>> reload on the next render.
>>>
>>> Sven
>>>
>>>
>>>  Just another caveat: what happens if during rending time, before call
>> AJAX
>> response is processed, the behavior is re-fired at client side? Wouldn't
>>   this produce a component not found exception?
>>
>>
>


-- 
Regards - Ernesto Reinaldo Barreiro
Antilia Soft
http://antiliasoft.com/ <http://antiliasoft.com/antilia>

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Sven Meier <sv...@meiers.net>.
Yeah, this is what I wrote in my first comment:

"Thus the next pending Ajax request comes through, since wicket-ajax's 
defaultPrecondition sees the markupId still present in the dom."

Perhaps we have to cancel out all scheduled callbacks for components 
which are replaced via Ajax.

Sven

On 01/17/2013 11:34 AM, Ernesto Reinaldo Barreiro wrote:
> Hi,
>
> On Thu, Jan 17, 2013 at 11:17 AM, Sven Meier <sv...@meiers.net> wrote:
>
>>          @Override
>>>          public void detach(Component component)
>>>          {
>>>                  AjaxRequestTarget target = component.getRequestCycle().**
>>> find(AjaxRequestTarget.class);
>>>                  if (target != null)
>>>                  {
>>>                          stop(target);
>>>                  }
>>>                  super.detach(component);
>>>          }
>>>
>>
>> A small nitpick:
>> Removing a component in an ART might not be the only reason why #detach()
>> is called. A developer might call component#detach() to force a LDM to
>> reload on the next render.
>>
>> Sven
>>
>>
> Just another caveat: what happens if during rending time, before call AJAX
> response is processed, the behavior is re-fired at client side? Wouldn't
>   this produce a component not found exception?
>


Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
Hi,

On Thu, Jan 17, 2013 at 11:17 AM, Sven Meier <sv...@meiers.net> wrote:

>         @Override
>>         public void detach(Component component)
>>         {
>>                 AjaxRequestTarget target = component.getRequestCycle().**
>> find(AjaxRequestTarget.class);
>>                 if (target != null)
>>                 {
>>                         stop(target);
>>                 }
>>                 super.detach(component);
>>         }
>>
>
>
> A small nitpick:
> Removing a component in an ART might not be the only reason why #detach()
> is called. A developer might call component#detach() to force a LDM to
> reload on the next render.
>
> Sven
>
>
Just another caveat: what happens if during rending time, before call AJAX
response is processed, the behavior is re-fired at client side? Wouldn't
 this produce a component not found exception?

-- 
Regards - Ernesto Reinaldo Barreiro

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Martin Grigorov <mg...@apache.org>.
I've attached a patch to WICKET-4959 that introduces
Behavior#onRemove(Component) - a callback that is called when a host
component is removed from its parent.

But there is still a chance for race , much lower than before but still
here .. :-/


On Thu, Jan 17, 2013 at 1:00 PM, Martin Grigorov <mg...@apache.org>wrote:

>
>
>
> On Thu, Jan 17, 2013 at 12:36 PM, Sven Meier <sv...@meiers.net> wrote:
>
>> I've used component#detach() when the code didn't have a reference to the
>> model.
>
>
>  component.getModel().detach() ?! :-)
>
>
>>
>>
>> > Maybe we need another callback method in Behavior
>>
>> A #removed() callback might be useful ... or overkill ;)
>>
>> Sven
>>
>>
>> On 01/17/2013 11:33 AM, Martin Grigorov wrote:
>>
>>> On Thu, Jan 17, 2013 at 12:17 PM, Sven Meier <sv...@meiers.net> wrote:
>>>
>>>           @Override
>>>>
>>>>>          public void detach(Component component)
>>>>>          {
>>>>>                  AjaxRequestTarget target =
>>>>> component.getRequestCycle().**
>>>>>
>>>>> find(AjaxRequestTarget.class);
>>>>>                  if (target != null)
>>>>>                  {
>>>>>                          stop(target);
>>>>>                  }
>>>>>                  super.detach(component);
>>>>>          }
>>>>>
>>>>>
>>>> A small nitpick:
>>>> Removing a component in an ART might not be the only reason why
>>>> #detach()
>>>> is called. A developer might call component#detach() to force a LDM to
>>>> reload on the next render.
>>>>
>>>
>>> Very good point!
>>> I'm not sure this is so small. I'd use model.detach() but one could
>>> detach
>>> the component ...
>>> Maybe we need another callback method in Behavior
>>>
>>>
>>>
>>>> Sven
>>>>
>>>>
>>>> On 01/17/2013 10:20 AM, mgrigorov@apache.org wrote:
>>>>
>>>>  WICKET-4959 Unproperly detached Behavior with TabbedPanels
>>>>>
>>>>> Stop the Ajax timer behavior when the behavior's component is removed
>>>>> from the tree
>>>>>
>>>>>
>>>>> Project: http://git-wip-us.apache.org/****repos/asf/wicket/repo<http://git-wip-us.apache.org/**repos/asf/wicket/repo>
>>>>> <http://**git-wip-us.apache.org/repos/**asf/wicket/repo<http://git-wip-us.apache.org/repos/asf/wicket/repo>
>>>>> >
>>>>> Commit: http://git-wip-us.apache.org/****repos/asf/wicket/commit/****
>>>>> 0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/commit/**0b78d759>
>>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**commit/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/commit/0b78d759>
>>>>> >
>>>>> Tree: http://git-wip-us.apache.org/****repos/asf/wicket/tree/**
>>>>> 0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/tree/0b78d759>
>>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**tree/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/tree/0b78d759>
>>>>> >
>>>>> Diff: http://git-wip-us.apache.org/****repos/asf/wicket/diff/**
>>>>> 0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/diff/0b78d759>
>>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**diff/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/diff/0b78d759>
>>>>> >
>>>>>
>>>>> Branch: refs/heads/master
>>>>> Commit: 0b78d759220c1b09abb0d47b500775****7bbfeb4e0c
>>>>>
>>>>> Parents: e37a9e1
>>>>> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>>>>> Authored: Thu Jan 17 11:18:22 2013 +0200
>>>>> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>>>>> Committed: Thu Jan 17 11:18:22 2013 +0200
>>>>>
>>>>> ------------------------------****----------------------------**--**
>>>>> ----------
>>>>>    .../wicket/ajax/****AbstractAjaxTimerBehavior.java     |   11
>>>>> +++++++++++
>>>>>
>>>>>    1 files changed, 11 insertions(+), 0 deletions(-)
>>>>> ------------------------------****----------------------------**--**
>>>>> ----------
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/****repos/asf/wicket/blob/**<http://git-wip-us.apache.org/**repos/asf/wicket/blob/**>
>>>>> 0b78d759/wicket-core/src/main/****java/org/apache/wicket/ajax/****
>>>>> AbstractAjaxTimerBehavior.**java<http://git-wip-us.apache.**
>>>>> org/repos/asf/wicket/blob/**0b78d759/wicket-core/src/main/**
>>>>> java/org/apache/wicket/ajax/**AbstractAjaxTimerBehavior.java<http://git-wip-us.apache.org/repos/asf/wicket/blob/0b78d759/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java>
>>>>> **>
>>>>> ------------------------------****----------------------------**--**
>>>>> ----------
>>>>> diff --git a/wicket-core/src/main/java/****org/apache/wicket/ajax/****
>>>>> AbstractAjaxTimerBehavior.java
>>>>> b/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>>> AbstractAjaxTimerBehavior.java
>>>>> index 83edeaa..a80921d 100644
>>>>> --- a/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>>> AbstractAjaxTimerBehavior.java
>>>>> +++ b/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>>>
>>>>> AbstractAjaxTimerBehavior.java
>>>>> @@ -194,4 +194,15 @@ public abstract class AbstractAjaxTimerBehavior
>>>>> extends AbstractDefaultAjaxBehav
>>>>>                  String timeoutHandle = getTimeoutHandle();
>>>>>                  target.prependJavaScript("**
>>>>> clearTimeout("+timeoutHandle+"****); delete "+timeoutHandle+";");
>>>>>
>>>>>          }
>>>>> +
>>>>> +       @Override
>>>>> +       public void detach(Component component)
>>>>> +       {
>>>>> +               AjaxRequestTarget target =
>>>>> component.getRequestCycle().**
>>>>>
>>>>> find(AjaxRequestTarget.class);
>>>>> +               if (target != null)
>>>>> +               {
>>>>> +                       stop(target);
>>>>> +               }
>>>>> +               super.detach(component);
>>>>> +       }
>>>>>    }
>>>>>
>>>>>
>>>>>
>>>
>>
>
>
> --
> Martin Grigorov
> jWeekend
> Training, Consulting, Development
> http://jWeekend.com <http://jweekend.com/>
>



-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Sven Meier <sv...@meiers.net>.
Lazy me ;)

On 01/17/2013 12:00 PM, Martin Grigorov wrote:
> On Thu, Jan 17, 2013 at 12:36 PM, Sven Meier <sv...@meiers.net> wrote:
>
>> I've used component#detach() when the code didn't have a reference to the
>> model.
>
> component.getModel().detach() ?! :-)
>
>
>>
>>> Maybe we need another callback method in Behavior
>> A #removed() callback might be useful ... or overkill ;)
>>
>> Sven
>>
>>
>> On 01/17/2013 11:33 AM, Martin Grigorov wrote:
>>
>>> On Thu, Jan 17, 2013 at 12:17 PM, Sven Meier <sv...@meiers.net> wrote:
>>>
>>>            @Override
>>>>>           public void detach(Component component)
>>>>>           {
>>>>>                   AjaxRequestTarget target =
>>>>> component.getRequestCycle().**
>>>>>
>>>>> find(AjaxRequestTarget.class);
>>>>>                   if (target != null)
>>>>>                   {
>>>>>                           stop(target);
>>>>>                   }
>>>>>                   super.detach(component);
>>>>>           }
>>>>>
>>>>>
>>>> A small nitpick:
>>>> Removing a component in an ART might not be the only reason why #detach()
>>>> is called. A developer might call component#detach() to force a LDM to
>>>> reload on the next render.
>>>>
>>> Very good point!
>>> I'm not sure this is so small. I'd use model.detach() but one could detach
>>> the component ...
>>> Maybe we need another callback method in Behavior
>>>
>>>
>>>
>>>> Sven
>>>>
>>>>
>>>> On 01/17/2013 10:20 AM, mgrigorov@apache.org wrote:
>>>>
>>>>   WICKET-4959 Unproperly detached Behavior with TabbedPanels
>>>>> Stop the Ajax timer behavior when the behavior's component is removed
>>>>> from the tree
>>>>>
>>>>>
>>>>> Project: http://git-wip-us.apache.org/****repos/asf/wicket/repo<http://git-wip-us.apache.org/**repos/asf/wicket/repo>
>>>>> <http://**git-wip-us.apache.org/repos/**asf/wicket/repo<http://git-wip-us.apache.org/repos/asf/wicket/repo>
>>>>> Commit: http://git-wip-us.apache.org/****repos/asf/wicket/commit/****
>>>>> 0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/commit/**0b78d759>
>>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**commit/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/commit/0b78d759>
>>>>> Tree: http://git-wip-us.apache.org/****repos/asf/wicket/tree/**0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/tree/0b78d759>
>>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**tree/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/tree/0b78d759>
>>>>> Diff: http://git-wip-us.apache.org/****repos/asf/wicket/diff/**0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/diff/0b78d759>
>>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**diff/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/diff/0b78d759>
>>>>> Branch: refs/heads/master
>>>>> Commit: 0b78d759220c1b09abb0d47b500775****7bbfeb4e0c
>>>>>
>>>>> Parents: e37a9e1
>>>>> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>>>>> Authored: Thu Jan 17 11:18:22 2013 +0200
>>>>> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>>>>> Committed: Thu Jan 17 11:18:22 2013 +0200
>>>>>
>>>>> ------------------------------****----------------------------**--**
>>>>> ----------
>>>>>     .../wicket/ajax/****AbstractAjaxTimerBehavior.java     |   11
>>>>> +++++++++++
>>>>>
>>>>>     1 files changed, 11 insertions(+), 0 deletions(-)
>>>>> ------------------------------****----------------------------**--**
>>>>> ----------
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/****repos/asf/wicket/blob/**<http://git-wip-us.apache.org/**repos/asf/wicket/blob/**>
>>>>> 0b78d759/wicket-core/src/main/****java/org/apache/wicket/ajax/****
>>>>> AbstractAjaxTimerBehavior.**java<http://git-wip-us.apache.**
>>>>> org/repos/asf/wicket/blob/**0b78d759/wicket-core/src/main/**
>>>>> java/org/apache/wicket/ajax/**AbstractAjaxTimerBehavior.java<http://git-wip-us.apache.org/repos/asf/wicket/blob/0b78d759/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java>
>>>>> **>
>>>>> ------------------------------****----------------------------**--**
>>>>> ----------
>>>>> diff --git a/wicket-core/src/main/java/****org/apache/wicket/ajax/****
>>>>> AbstractAjaxTimerBehavior.java
>>>>> b/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>>> AbstractAjaxTimerBehavior.java
>>>>> index 83edeaa..a80921d 100644
>>>>> --- a/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>>> AbstractAjaxTimerBehavior.java
>>>>> +++ b/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>>>
>>>>> AbstractAjaxTimerBehavior.java
>>>>> @@ -194,4 +194,15 @@ public abstract class AbstractAjaxTimerBehavior
>>>>> extends AbstractDefaultAjaxBehav
>>>>>                   String timeoutHandle = getTimeoutHandle();
>>>>>                   target.prependJavaScript("**
>>>>> clearTimeout("+timeoutHandle+"****); delete "+timeoutHandle+";");
>>>>>
>>>>>           }
>>>>> +
>>>>> +       @Override
>>>>> +       public void detach(Component component)
>>>>> +       {
>>>>> +               AjaxRequestTarget target =
>>>>> component.getRequestCycle().**
>>>>>
>>>>> find(AjaxRequestTarget.class);
>>>>> +               if (target != null)
>>>>> +               {
>>>>> +                       stop(target);
>>>>> +               }
>>>>> +               super.detach(component);
>>>>> +       }
>>>>>     }
>>>>>
>>>>>
>>>>>
>


Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Martin Grigorov <mg...@apache.org>.
On Thu, Jan 17, 2013 at 12:36 PM, Sven Meier <sv...@meiers.net> wrote:

> I've used component#detach() when the code didn't have a reference to the
> model.


component.getModel().detach() ?! :-)


>
>
> > Maybe we need another callback method in Behavior
>
> A #removed() callback might be useful ... or overkill ;)
>
> Sven
>
>
> On 01/17/2013 11:33 AM, Martin Grigorov wrote:
>
>> On Thu, Jan 17, 2013 at 12:17 PM, Sven Meier <sv...@meiers.net> wrote:
>>
>>           @Override
>>>
>>>>          public void detach(Component component)
>>>>          {
>>>>                  AjaxRequestTarget target =
>>>> component.getRequestCycle().**
>>>>
>>>> find(AjaxRequestTarget.class);
>>>>                  if (target != null)
>>>>                  {
>>>>                          stop(target);
>>>>                  }
>>>>                  super.detach(component);
>>>>          }
>>>>
>>>>
>>> A small nitpick:
>>> Removing a component in an ART might not be the only reason why #detach()
>>> is called. A developer might call component#detach() to force a LDM to
>>> reload on the next render.
>>>
>>
>> Very good point!
>> I'm not sure this is so small. I'd use model.detach() but one could detach
>> the component ...
>> Maybe we need another callback method in Behavior
>>
>>
>>
>>> Sven
>>>
>>>
>>> On 01/17/2013 10:20 AM, mgrigorov@apache.org wrote:
>>>
>>>  WICKET-4959 Unproperly detached Behavior with TabbedPanels
>>>>
>>>> Stop the Ajax timer behavior when the behavior's component is removed
>>>> from the tree
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/****repos/asf/wicket/repo<http://git-wip-us.apache.org/**repos/asf/wicket/repo>
>>>> <http://**git-wip-us.apache.org/repos/**asf/wicket/repo<http://git-wip-us.apache.org/repos/asf/wicket/repo>
>>>> >
>>>> Commit: http://git-wip-us.apache.org/****repos/asf/wicket/commit/****
>>>> 0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/commit/**0b78d759>
>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**commit/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/commit/0b78d759>
>>>> >
>>>> Tree: http://git-wip-us.apache.org/****repos/asf/wicket/tree/**0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/tree/0b78d759>
>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**tree/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/tree/0b78d759>
>>>> >
>>>> Diff: http://git-wip-us.apache.org/****repos/asf/wicket/diff/**0b78d759<http://git-wip-us.apache.org/**repos/asf/wicket/diff/0b78d759>
>>>> <http://git-wip-us.**apache.org/repos/asf/wicket/**diff/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/diff/0b78d759>
>>>> >
>>>>
>>>> Branch: refs/heads/master
>>>> Commit: 0b78d759220c1b09abb0d47b500775****7bbfeb4e0c
>>>>
>>>> Parents: e37a9e1
>>>> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>>>> Authored: Thu Jan 17 11:18:22 2013 +0200
>>>> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>>>> Committed: Thu Jan 17 11:18:22 2013 +0200
>>>>
>>>> ------------------------------****----------------------------**--**
>>>> ----------
>>>>    .../wicket/ajax/****AbstractAjaxTimerBehavior.java     |   11
>>>> +++++++++++
>>>>
>>>>    1 files changed, 11 insertions(+), 0 deletions(-)
>>>> ------------------------------****----------------------------**--**
>>>> ----------
>>>>
>>>>
>>>> http://git-wip-us.apache.org/****repos/asf/wicket/blob/**<http://git-wip-us.apache.org/**repos/asf/wicket/blob/**>
>>>> 0b78d759/wicket-core/src/main/****java/org/apache/wicket/ajax/****
>>>> AbstractAjaxTimerBehavior.**java<http://git-wip-us.apache.**
>>>> org/repos/asf/wicket/blob/**0b78d759/wicket-core/src/main/**
>>>> java/org/apache/wicket/ajax/**AbstractAjaxTimerBehavior.java<http://git-wip-us.apache.org/repos/asf/wicket/blob/0b78d759/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java>
>>>> **>
>>>> ------------------------------****----------------------------**--**
>>>> ----------
>>>> diff --git a/wicket-core/src/main/java/****org/apache/wicket/ajax/****
>>>> AbstractAjaxTimerBehavior.java
>>>> b/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>> AbstractAjaxTimerBehavior.java
>>>> index 83edeaa..a80921d 100644
>>>> --- a/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>> AbstractAjaxTimerBehavior.java
>>>> +++ b/wicket-core/src/main/java/****org/apache/wicket/ajax/**
>>>>
>>>> AbstractAjaxTimerBehavior.java
>>>> @@ -194,4 +194,15 @@ public abstract class AbstractAjaxTimerBehavior
>>>> extends AbstractDefaultAjaxBehav
>>>>                  String timeoutHandle = getTimeoutHandle();
>>>>                  target.prependJavaScript("**
>>>> clearTimeout("+timeoutHandle+"****); delete "+timeoutHandle+";");
>>>>
>>>>          }
>>>> +
>>>> +       @Override
>>>> +       public void detach(Component component)
>>>> +       {
>>>> +               AjaxRequestTarget target =
>>>> component.getRequestCycle().**
>>>>
>>>> find(AjaxRequestTarget.class);
>>>> +               if (target != null)
>>>> +               {
>>>> +                       stop(target);
>>>> +               }
>>>> +               super.detach(component);
>>>> +       }
>>>>    }
>>>>
>>>>
>>>>
>>
>


-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>

Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Sven Meier <sv...@meiers.net>.
I've used component#detach() when the code didn't have a reference to 
the model.

 > Maybe we need another callback method in Behavior

A #removed() callback might be useful ... or overkill ;)

Sven

On 01/17/2013 11:33 AM, Martin Grigorov wrote:
> On Thu, Jan 17, 2013 at 12:17 PM, Sven Meier <sv...@meiers.net> wrote:
>
>>          @Override
>>>          public void detach(Component component)
>>>          {
>>>                  AjaxRequestTarget target = component.getRequestCycle().**
>>> find(AjaxRequestTarget.class);
>>>                  if (target != null)
>>>                  {
>>>                          stop(target);
>>>                  }
>>>                  super.detach(component);
>>>          }
>>>
>>
>> A small nitpick:
>> Removing a component in an ART might not be the only reason why #detach()
>> is called. A developer might call component#detach() to force a LDM to
>> reload on the next render.
>
> Very good point!
> I'm not sure this is so small. I'd use model.detach() but one could detach
> the component ...
> Maybe we need another callback method in Behavior
>
>
>>
>> Sven
>>
>>
>> On 01/17/2013 10:20 AM, mgrigorov@apache.org wrote:
>>
>>> WICKET-4959 Unproperly detached Behavior with TabbedPanels
>>>
>>> Stop the Ajax timer behavior when the behavior's component is removed
>>> from the tree
>>>
>>>
>>> Project: http://git-wip-us.apache.org/**repos/asf/wicket/repo<http://git-wip-us.apache.org/repos/asf/wicket/repo>
>>> Commit: http://git-wip-us.apache.org/**repos/asf/wicket/commit/**0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/commit/0b78d759>
>>> Tree: http://git-wip-us.apache.org/**repos/asf/wicket/tree/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/tree/0b78d759>
>>> Diff: http://git-wip-us.apache.org/**repos/asf/wicket/diff/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/diff/0b78d759>
>>>
>>> Branch: refs/heads/master
>>> Commit: 0b78d759220c1b09abb0d47b500775**7bbfeb4e0c
>>> Parents: e37a9e1
>>> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>>> Authored: Thu Jan 17 11:18:22 2013 +0200
>>> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>>> Committed: Thu Jan 17 11:18:22 2013 +0200
>>>
>>> ------------------------------**------------------------------**
>>> ----------
>>>    .../wicket/ajax/**AbstractAjaxTimerBehavior.java     |   11 +++++++++++
>>>    1 files changed, 11 insertions(+), 0 deletions(-)
>>> ------------------------------**------------------------------**
>>> ----------
>>>
>>>
>>> http://git-wip-us.apache.org/**repos/asf/wicket/blob/**
>>> 0b78d759/wicket-core/src/main/**java/org/apache/wicket/ajax/**
>>> AbstractAjaxTimerBehavior.java<http://git-wip-us.apache.org/repos/asf/wicket/blob/0b78d759/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java>
>>> ------------------------------**------------------------------**
>>> ----------
>>> diff --git a/wicket-core/src/main/java/**org/apache/wicket/ajax/**AbstractAjaxTimerBehavior.java
>>> b/wicket-core/src/main/java/**org/apache/wicket/ajax/**
>>> AbstractAjaxTimerBehavior.java
>>> index 83edeaa..a80921d 100644
>>> --- a/wicket-core/src/main/java/**org/apache/wicket/ajax/**
>>> AbstractAjaxTimerBehavior.java
>>> +++ b/wicket-core/src/main/java/**org/apache/wicket/ajax/**
>>> AbstractAjaxTimerBehavior.java
>>> @@ -194,4 +194,15 @@ public abstract class AbstractAjaxTimerBehavior
>>> extends AbstractDefaultAjaxBehav
>>>                  String timeoutHandle = getTimeoutHandle();
>>>                  target.prependJavaScript("**
>>> clearTimeout("+timeoutHandle+"**); delete "+timeoutHandle+";");
>>>          }
>>> +
>>> +       @Override
>>> +       public void detach(Component component)
>>> +       {
>>> +               AjaxRequestTarget target = component.getRequestCycle().**
>>> find(AjaxRequestTarget.class);
>>> +               if (target != null)
>>> +               {
>>> +                       stop(target);
>>> +               }
>>> +               super.detach(component);
>>> +       }
>>>    }
>>>
>>>
>


Re: [1/2] git commit: WICKET-4959 Unproperly detached Behavior with TabbedPanels

Posted by Martin Grigorov <mg...@apache.org>.
On Thu, Jan 17, 2013 at 12:17 PM, Sven Meier <sv...@meiers.net> wrote:

>         @Override
>>         public void detach(Component component)
>>         {
>>                 AjaxRequestTarget target = component.getRequestCycle().**
>> find(AjaxRequestTarget.class);
>>                 if (target != null)
>>                 {
>>                         stop(target);
>>                 }
>>                 super.detach(component);
>>         }
>>
>
>
> A small nitpick:
> Removing a component in an ART might not be the only reason why #detach()
> is called. A developer might call component#detach() to force a LDM to
> reload on the next render.


Very good point!
I'm not sure this is so small. I'd use model.detach() but one could detach
the component ...
Maybe we need another callback method in Behavior


>
>
> Sven
>
>
> On 01/17/2013 10:20 AM, mgrigorov@apache.org wrote:
>
>> WICKET-4959 Unproperly detached Behavior with TabbedPanels
>>
>> Stop the Ajax timer behavior when the behavior's component is removed
>> from the tree
>>
>>
>> Project: http://git-wip-us.apache.org/**repos/asf/wicket/repo<http://git-wip-us.apache.org/repos/asf/wicket/repo>
>> Commit: http://git-wip-us.apache.org/**repos/asf/wicket/commit/**0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/commit/0b78d759>
>> Tree: http://git-wip-us.apache.org/**repos/asf/wicket/tree/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/tree/0b78d759>
>> Diff: http://git-wip-us.apache.org/**repos/asf/wicket/diff/0b78d759<http://git-wip-us.apache.org/repos/asf/wicket/diff/0b78d759>
>>
>> Branch: refs/heads/master
>> Commit: 0b78d759220c1b09abb0d47b500775**7bbfeb4e0c
>> Parents: e37a9e1
>> Author: Martin Tzvetanov Grigorov <mg...@apache.org>
>> Authored: Thu Jan 17 11:18:22 2013 +0200
>> Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
>> Committed: Thu Jan 17 11:18:22 2013 +0200
>>
>> ------------------------------**------------------------------**
>> ----------
>>   .../wicket/ajax/**AbstractAjaxTimerBehavior.java     |   11 +++++++++++
>>   1 files changed, 11 insertions(+), 0 deletions(-)
>> ------------------------------**------------------------------**
>> ----------
>>
>>
>> http://git-wip-us.apache.org/**repos/asf/wicket/blob/**
>> 0b78d759/wicket-core/src/main/**java/org/apache/wicket/ajax/**
>> AbstractAjaxTimerBehavior.java<http://git-wip-us.apache.org/repos/asf/wicket/blob/0b78d759/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java>
>> ------------------------------**------------------------------**
>> ----------
>> diff --git a/wicket-core/src/main/java/**org/apache/wicket/ajax/**AbstractAjaxTimerBehavior.java
>> b/wicket-core/src/main/java/**org/apache/wicket/ajax/**
>> AbstractAjaxTimerBehavior.java
>> index 83edeaa..a80921d 100644
>> --- a/wicket-core/src/main/java/**org/apache/wicket/ajax/**
>> AbstractAjaxTimerBehavior.java
>> +++ b/wicket-core/src/main/java/**org/apache/wicket/ajax/**
>> AbstractAjaxTimerBehavior.java
>> @@ -194,4 +194,15 @@ public abstract class AbstractAjaxTimerBehavior
>> extends AbstractDefaultAjaxBehav
>>                 String timeoutHandle = getTimeoutHandle();
>>                 target.prependJavaScript("**
>> clearTimeout("+timeoutHandle+"**); delete "+timeoutHandle+";");
>>         }
>> +
>> +       @Override
>> +       public void detach(Component component)
>> +       {
>> +               AjaxRequestTarget target = component.getRequestCycle().**
>> find(AjaxRequestTarget.class);
>> +               if (target != null)
>> +               {
>> +                       stop(target);
>> +               }
>> +               super.detach(component);
>> +       }
>>   }
>>
>>
>


-- 
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com <http://jweekend.com/>