You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martijn Dashorst <ma...@gmail.com> on 2006/11/30 16:03:47 UTC

Detach behavior of ajax requests

Currently ajax requests don't detach anything. I have created a unit
test that checks the detach behavior of the ajax request, but I am not
sure what we should detach. Even so, in the AjaxRequestTarget we don't
have the full API available to properly detach all components that
participated in the request.

For instance, the model of the page could be attached without the page
knowing about it because of model inheritance. Currently the detach of
Page checks if the page is 'attached'.

I think that we should always call internalDetach, detach,
detachModels, detachBehaviors, and onDetach for all components
regardless whether they are actually attached.

Martijn

-- 
<a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
for <a href="http://www.thebeststuffintheworld.com/stuff/wicket">Wicket</a>
at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
the World!</a>

Re: Re: Detach behavior of ajax requests

Posted by Martijn Dashorst <ma...@gmail.com>.
In my unit test it is called only once ;-)

But perhaps we need another unit test to exercise this bug as well.

Martijn

On 12/1/06, Johan Compagner <jc...@gmail.com> wrote:
> ehmmm
> I just added that test because of another open bug!!
> page.detach() called twice......
>
> And i believe i already did said this would then happen..
>
> But you can remove it again. But then do know that we can't say detach is
> only called once!
>
> johan
>
>
> On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
> >
> > I propose to remove the attached check in page.internalDetach (line
> > 287) and to remove
> > - the internalDetach() call in component.renderComponent() (line 1658)
> > - the detachModels() call to behavior in
> > AjaxRequestTarget.respondHeaderContributions (lines 533 and 543)
> >
> > Then we still detach everything, but only once (the behavior still
> > gets detached twice during the ajax request). Because the page call
> > still detaches everything we still perform correct detach behavior,
> > and we don't have scattered detach logic throughout the framework.
> >
> > Martijn
> >
> > On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
> > > Currently ajax requests don't detach anything. I have created a unit
> > > test that checks the detach behavior of the ajax request, but I am not
> > > sure what we should detach. Even so, in the AjaxRequestTarget we don't
> > > have the full API available to properly detach all components that
> > > participated in the request.
> > >
> > > For instance, the model of the page could be attached without the page
> > > knowing about it because of model inheritance. Currently the detach of
> > > Page checks if the page is 'attached'.
> > >
> > > I think that we should always call internalDetach, detach,
> > > detachModels, detachBehaviors, and onDetach for all components
> > > regardless whether they are actually attached.
> > >
> > > Martijn
> > >
> > > --
> > > <a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
> > > for <a href="http://www.thebeststuffintheworld.com/stuff/wicket
> > ">Wicket</a>
> > > at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
> > > the World!</a>
> > >
> >
> >
> > --
> > <a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
> > for <a href="http://www.thebeststuffintheworld.com/stuff/wicket
> > ">Wicket</a>
> > at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
> > the World!</a>
> >
>
>


-- 
<a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
for <a href="http://www.thebeststuffintheworld.com/stuff/wicket">Wicket</a>
at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
the World!</a>

Re: Detach behavior of ajax requests

Posted by Johan Compagner <jc...@gmail.com>.
ehmmm
I just added that test because of another open bug!!
page.detach() called twice......

And i believe i already did said this would then happen..

But you can remove it again. But then do know that we can't say detach is
only called once!

johan


On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
>
> I propose to remove the attached check in page.internalDetach (line
> 287) and to remove
> - the internalDetach() call in component.renderComponent() (line 1658)
> - the detachModels() call to behavior in
> AjaxRequestTarget.respondHeaderContributions (lines 533 and 543)
>
> Then we still detach everything, but only once (the behavior still
> gets detached twice during the ajax request). Because the page call
> still detaches everything we still perform correct detach behavior,
> and we don't have scattered detach logic throughout the framework.
>
> Martijn
>
> On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
> > Currently ajax requests don't detach anything. I have created a unit
> > test that checks the detach behavior of the ajax request, but I am not
> > sure what we should detach. Even so, in the AjaxRequestTarget we don't
> > have the full API available to properly detach all components that
> > participated in the request.
> >
> > For instance, the model of the page could be attached without the page
> > knowing about it because of model inheritance. Currently the detach of
> > Page checks if the page is 'attached'.
> >
> > I think that we should always call internalDetach, detach,
> > detachModels, detachBehaviors, and onDetach for all components
> > regardless whether they are actually attached.
> >
> > Martijn
> >
> > --
> > <a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
> > for <a href="http://www.thebeststuffintheworld.com/stuff/wicket
> ">Wicket</a>
> > at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
> > the World!</a>
> >
>
>
> --
> <a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
> for <a href="http://www.thebeststuffintheworld.com/stuff/wicket
> ">Wicket</a>
> at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
> the World!</a>
>

Re: Detach behavior of ajax requests

Posted by Erik van Oosten <e....@chello.nl>.
Could this be backported to 1.2 as well?

    Erik.

Martijn Dashorst schreef:
> This change gives me a working unit test for 1.3 (ComponentTest).
>
> Martijn
>
> On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
>> I propose to remove the attached check in page.internalDetach (line
>> 287) and to remove
>> - the internalDetach() call in component.renderComponent() (line 1658)
>> - the detachModels() call to behavior in
>> AjaxRequestTarget.respondHeaderContributions (lines 533 and 543)
>>
>> Then we still detach everything, but only once (the behavior still
>> gets detached twice during the ajax request). Because the page call
>> still detaches everything we still perform correct detach behavior,
>> and we don't have scattered detach logic throughout the framework.
>>
>> Martijn

-- 
Erik van Oosten
http://www.day-to-day-stuff.blogspot.com/


Re: Re: Detach behavior of ajax requests

Posted by Martijn Dashorst <ma...@gmail.com>.
This change gives me a working unit test for 1.3 (ComponentTest).

Martijn

On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
> I propose to remove the attached check in page.internalDetach (line
> 287) and to remove
> - the internalDetach() call in component.renderComponent() (line 1658)
> - the detachModels() call to behavior in
> AjaxRequestTarget.respondHeaderContributions (lines 533 and 543)
>
> Then we still detach everything, but only once (the behavior still
> gets detached twice during the ajax request). Because the page call
> still detaches everything we still perform correct detach behavior,
> and we don't have scattered detach logic throughout the framework.
>
> Martijn
>
> On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
> > Currently ajax requests don't detach anything. I have created a unit
> > test that checks the detach behavior of the ajax request, but I am not
> > sure what we should detach. Even so, in the AjaxRequestTarget we don't
> > have the full API available to properly detach all components that
> > participated in the request.
> >
> > For instance, the model of the page could be attached without the page
> > knowing about it because of model inheritance. Currently the detach of
> > Page checks if the page is 'attached'.
> >
> > I think that we should always call internalDetach, detach,
> > detachModels, detachBehaviors, and onDetach for all components
> > regardless whether they are actually attached.
> >
> > Martijn
> >
> > --
> > <a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
> > for <a href="http://www.thebeststuffintheworld.com/stuff/wicket">Wicket</a>
> > at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
> > the World!</a>
> >
>
>
> --
> <a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
> for <a href="http://www.thebeststuffintheworld.com/stuff/wicket">Wicket</a>
> at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
> the World!</a>
>


-- 
<a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
for <a href="http://www.thebeststuffintheworld.com/stuff/wicket">Wicket</a>
at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
the World!</a>

Re: Detach behavior of ajax requests

Posted by Martijn Dashorst <ma...@gmail.com>.
I propose to remove the attached check in page.internalDetach (line
287) and to remove
- the internalDetach() call in component.renderComponent() (line 1658)
- the detachModels() call to behavior in
AjaxRequestTarget.respondHeaderContributions (lines 533 and 543)

Then we still detach everything, but only once (the behavior still
gets detached twice during the ajax request). Because the page call
still detaches everything we still perform correct detach behavior,
and we don't have scattered detach logic throughout the framework.

Martijn

On 11/30/06, Martijn Dashorst <ma...@gmail.com> wrote:
> Currently ajax requests don't detach anything. I have created a unit
> test that checks the detach behavior of the ajax request, but I am not
> sure what we should detach. Even so, in the AjaxRequestTarget we don't
> have the full API available to properly detach all components that
> participated in the request.
>
> For instance, the model of the page could be attached without the page
> knowing about it because of model inheritance. Currently the detach of
> Page checks if the page is 'attached'.
>
> I think that we should always call internalDetach, detach,
> detachModels, detachBehaviors, and onDetach for all components
> regardless whether they are actually attached.
>
> Martijn
>
> --
> <a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
> for <a href="http://www.thebeststuffintheworld.com/stuff/wicket">Wicket</a>
> at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
> the World!</a>
>


-- 
<a href="http://www.thebeststuffintheworld.com/vote_for/wicket">Vote</a>
for <a href="http://www.thebeststuffintheworld.com/stuff/wicket">Wicket</a>
at the <a href="http://www.thebeststuffintheworld.com/">Best Stuff in
the World!</a>

Re: Detach behavior of ajax requests

Posted by Matej Knopp <ma...@knopp.sk>.
I think so too. IMHO we should detach page after every request
(including ajax). Detaching page should detach all components on page
too of course.

-Matej

Martijn Dashorst wrote:
> Currently ajax requests don't detach anything. I have created a unit
> test that checks the detach behavior of the ajax request, but I am not
> sure what we should detach. Even so, in the AjaxRequestTarget we don't
> have the full API available to properly detach all components that
> participated in the request.
> 
> For instance, the model of the page could be attached without the page
> knowing about it because of model inheritance. Currently the detach of
> Page checks if the page is 'attached'.
> 
> I think that we should always call internalDetach, detach,
> detachModels, detachBehaviors, and onDetach for all components
> regardless whether they are actually attached.
> 
> Martijn
>