Attach refactor severely broke 1.x

[Matej Knopp <ma...@gmail.com>]

Hi

1.x is broken.
Consider following scenario
Have a list view with loadable detachable model and an ajaxlink on list item.
When you click the ajaxlink, the list item model calls
listview.getmodelobject, which loads the list. But it doesn't attach
the listView, thus the listview model is not detached. IIRC before the
refactor, the listview was both attached and detached (the list item
attached the listview).

This thing also breaks AjaxTree, where the tree is not attached when
items are expanded/collapsed.

Igor?

-Matej

Re: Attach refactor severely broke 1.x

[Matej Knopp <ma...@gmail.com>]

Created jira issue https://issues.apache.org/jira/browse/WICKET-418

On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> I looks like the targets already detach the page, thus i don't
> understand why also ajaxrequesttarget does it.
>
> I'm not sure what do you mean by "AjaxRequestTarget knows which
> components were added to the target:...". All components need to be
> detached, not just those added to ajax
> request target. The doesn't only cascade down the hierarchy. I've got
> case of ajax tree where i only add tree item to the target, but i need
> entire tree to be attached. If ajaxrequesttarget only detached
> components added to it this would be a serious problem.
>
> -Matej
>
> On 3/25/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> > > Yes, i think we should detach the component models even if the
> > > components are not detached (which is not case with current
> > > Page.onDetach).
> > >
> > > And I also agree that request cycle should be responsible for detaching.
> >
> > The problem may be quite complicated: what components did take part in
> > the request?
> >
> > I think only the targets know which components were part of the request:
> >
> > AjaxRequestTarget knows which components were added to the target:
> > those are attached (and their models). The PageRequestTarget ditto...
> > So I think triggering the detach should come from the request cycle,
> > and afaik that already happens, but the actual detachment should be
> > delegated to the targets of the request (the whole stack should be
> > detached).
> >
> > Martijn
> >
> > --
> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > Join the wicket community at irc.freenode.net: ##wicket
> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > http://wicketframework.org
> >
>

Re: Attach refactor severely broke 1.x

[Matej Knopp <ma...@gmail.com>]

I looks like the targets already detach the page, thus i don't
understand why also ajaxrequesttarget does it.

I'm not sure what do you mean by "AjaxRequestTarget knows which
components were added to the target:...". All components need to be
detached, not just those added to ajax
request target. The doesn't only cascade down the hierarchy. I've got
case of ajax tree where i only add tree item to the target, but i need
entire tree to be attached. If ajaxrequesttarget only detached
components added to it this would be a serious problem.

-Matej

On 3/25/07, Martijn Dashorst <ma...@gmail.com> wrote:
> On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> > Yes, i think we should detach the component models even if the
> > components are not detached (which is not case with current
> > Page.onDetach).
> >
> > And I also agree that request cycle should be responsible for detaching.
>
> The problem may be quite complicated: what components did take part in
> the request?
>
> I think only the targets know which components were part of the request:
>
> AjaxRequestTarget knows which components were added to the target:
> those are attached (and their models). The PageRequestTarget ditto...
> So I think triggering the detach should come from the request cycle,
> and afaik that already happens, but the actual detachment should be
> delegated to the targets of the request (the whole stack should be
> detached).
>
> Martijn
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Attach refactor severely broke 1.x

[Martijn Dashorst <ma...@gmail.com>]

On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> Yes, i think we should detach the component models even if the
> components are not detached (which is not case with current
> Page.onDetach).
>
> And I also agree that request cycle should be responsible for detaching.

The problem may be quite complicated: what components did take part in
the request?

I think only the targets know which components were part of the request:

AjaxRequestTarget knows which components were added to the target:
those are attached (and their models). The PageRequestTarget ditto...
So I think triggering the detach should come from the request cycle,
and afaik that already happens, but the actual detachment should be
delegated to the targets of the request (the whole stack should be
detached).

Martijn

-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Attach refactor severely broke 1.x

[Matej Knopp <ma...@gmail.com>]

Yes, i think we should detach the component models even if the
components are not detached (which is not case with current
Page.onDetach).

And I also agree that request cycle should be responsible for detaching.

-Matej

On 3/25/07, Martijn Dashorst <ma...@gmail.com> wrote:
> On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> > Well, after further investigation: The problem with listview is that
> > page.detachModels is not called. AjaxRequestTarget calls page.detach,
> > however the page is not really attached so the detach method doesn't
> > call detachModels.
> >
> > Plus I'm not sure if calling page.detach is AjaxRequestTarget's
> > responsibility. What will happen if there is no component added to
> > ajax request target? Will it not be detached? (it probably will as the
> > session store should detach the page but anyway...)
>
> This touches on a subject raised earlier: the detachment is broken, at
> least, it is done in 24 different places, whereas it should be part of
> the requestcycle, where all pages touched in a request should be
> detached.
>
> The problem is that a component can be detached, but its model
> attached (nested LDM inside a CPM, accessed through a self updating
> label).
>
> Martijn
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Attach refactor severely broke 1.x

[Martijn Dashorst <ma...@gmail.com>]

On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> Well, after further investigation: The problem with listview is that
> page.detachModels is not called. AjaxRequestTarget calls page.detach,
> however the page is not really attached so the detach method doesn't
> call detachModels.
>
> Plus I'm not sure if calling page.detach is AjaxRequestTarget's
> responsibility. What will happen if there is no component added to
> ajax request target? Will it not be detached? (it probably will as the
> session store should detach the page but anyway...)

This touches on a subject raised earlier: the detachment is broken, at
least, it is done in 24 different places, whereas it should be part of
the requestcycle, where all pages touched in a request should be
detached.

The problem is that a component can be detached, but its model
attached (nested LDM inside a CPM, accessed through a self updating
label).

Martijn

-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Attach refactor severely broke 1.x

[Matej Knopp <ma...@gmail.com>]

Well, after further investigation: The problem with listview is that
page.detachModels is not called. AjaxRequestTarget calls page.detach,
however the page is not really attached so the detach method doesn't
call detachModels.

Plus I'm not sure if calling page.detach is AjaxRequestTarget's
responsibility. What will happen if there is no component added to
ajax request target? Will it not be detached? (it probably will as the
session store should detach the page but anyway...)

-Matej

On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> Now I'm no longer sure. Did this work actually before the refacotr?
>
> -Matej
>
> On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> > Hi
> >
> > 1.x is broken.
> > Consider following scenario
> > Have a list view with loadable detachable model and an ajaxlink on list item.
> > When you click the ajaxlink, the list item model calls
> > listview.getmodelobject, which loads the list. But it doesn't attach
> > the listView, thus the listview model is not detached. IIRC before the
> > refactor, the listview was both attached and detached (the list item
> > attached the listview).
> >
> > This thing also breaks AjaxTree, where the tree is not attached when
> > items are expanded/collapsed.
> >
> > Igor?
> >
> > -Matej
> >
>

Re: Attach refactor severely broke 1.x

[Matej Knopp <ma...@gmail.com>]

Now I'm no longer sure. Did this work actually before the refacotr?

-Matej

On 3/25/07, Matej Knopp <ma...@gmail.com> wrote:
> Hi
>
> 1.x is broken.
> Consider following scenario
> Have a list view with loadable detachable model and an ajaxlink on list item.
> When you click the ajaxlink, the list item model calls
> listview.getmodelobject, which loads the list. But it doesn't attach
> the listView, thus the listview model is not detached. IIRC before the
> refactor, the listview was both attached and detached (the list item
> attached the listview).
>
> This thing also breaks AjaxTree, where the tree is not attached when
> items are expanded/collapsed.
>
> Igor?
>
> -Matej
>