Re: Add onAddToPage() to Component

2014-08-26 Thread Martin Grigorov
https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677 looks
quite simpler now (without all the formating noise) !

+1 to merge it to wicket 6.x from me!

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Tue, Aug 26, 2014 at 8:56 PM, Martin Grigorov 
wrote:

> + private static final int FLAG_RE_ADDED = 0x40;
>
> maybe use
> /** Reserved subclass-definable flag bit */
> private static final int FLAG_NOTUSED7 = 0x4;
> ?
> it is not used at the moment
>
> keep the constants ordered so it is easy to find what is the next
> available value for the next time
>
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
>
> On Mon, Aug 25, 2014 at 10:22 AM, Carl-Eric Menzel 
> wrote:
>
>> I've applied the changes you suggested, they seem to work nicely. Only
>> in the WICKET-5677 branch based on 6.x so far, I'll cherry-pick it to
>> master once we agree that we can integrate this.
>>
>> Carl-Eric
>>
>> On Thu, 21 Aug 2014 23:30:45 +0200
>> Carl-Eric Menzel  wrote:
>>
>> > On Wed, 20 Aug 2014 15:26:45 +0300
>> > Martin Grigorov  wrote:
>> >
>> > > Carl-Eric,
>> > >
>> > > could you please re-format the related classes in wicket-6.x branch
>> > > so all noise like
>> > >
>> https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677#diff-bfa8bce2a5e14af19e42011a5e2d4c68L4380
>> > > disappear ?
>> > > Just open the same classes in wicket-6.x branch, format them,
>> > > commit+push. Cherry-pick in master.
>> >
>> > Will do.
>> >
>> > > I've added a comment on one of the commits in GitHub but so far I
>> > > don't received a notification about it. Did you receive a
>> > > notification about
>> > >
>> https://github.com/apache/wicket/commit/4703c1ae1af91fdcca44e34edbeeb8bbaeb63b6c#commitcomment-7456068
>> ?
>> >
>> > I didn't get a notification. But I'll have a look.
>> >
>> > Carl-Eric
>>
>>
>


Re: Add onAddToPage() to Component

2014-08-26 Thread Martin Grigorov
+ private static final int FLAG_RE_ADDED = 0x40;

maybe use
/** Reserved subclass-definable flag bit */
private static final int FLAG_NOTUSED7 = 0x4;
?
it is not used at the moment

keep the constants ordered so it is easy to find what is the next available
value for the next time


Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Mon, Aug 25, 2014 at 10:22 AM, Carl-Eric Menzel 
wrote:

> I've applied the changes you suggested, they seem to work nicely. Only
> in the WICKET-5677 branch based on 6.x so far, I'll cherry-pick it to
> master once we agree that we can integrate this.
>
> Carl-Eric
>
> On Thu, 21 Aug 2014 23:30:45 +0200
> Carl-Eric Menzel  wrote:
>
> > On Wed, 20 Aug 2014 15:26:45 +0300
> > Martin Grigorov  wrote:
> >
> > > Carl-Eric,
> > >
> > > could you please re-format the related classes in wicket-6.x branch
> > > so all noise like
> > >
> https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677#diff-bfa8bce2a5e14af19e42011a5e2d4c68L4380
> > > disappear ?
> > > Just open the same classes in wicket-6.x branch, format them,
> > > commit+push. Cherry-pick in master.
> >
> > Will do.
> >
> > > I've added a comment on one of the commits in GitHub but so far I
> > > don't received a notification about it. Did you receive a
> > > notification about
> > >
> https://github.com/apache/wicket/commit/4703c1ae1af91fdcca44e34edbeeb8bbaeb63b6c#commitcomment-7456068
> ?
> >
> > I didn't get a notification. But I'll have a look.
> >
> > Carl-Eric
>
>


Re: Add onAddToPage() to Component

2014-08-25 Thread Carl-Eric Menzel
I've applied the changes you suggested, they seem to work nicely. Only
in the WICKET-5677 branch based on 6.x so far, I'll cherry-pick it to
master once we agree that we can integrate this.

Carl-Eric

On Thu, 21 Aug 2014 23:30:45 +0200
Carl-Eric Menzel  wrote:

> On Wed, 20 Aug 2014 15:26:45 +0300
> Martin Grigorov  wrote:
> 
> > Carl-Eric,
> > 
> > could you please re-format the related classes in wicket-6.x branch
> > so all noise like
> > https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677#diff-bfa8bce2a5e14af19e42011a5e2d4c68L4380
> > disappear ?
> > Just open the same classes in wicket-6.x branch, format them,
> > commit+push. Cherry-pick in master.
> 
> Will do.
> 
> > I've added a comment on one of the commits in GitHub but so far I
> > don't received a notification about it. Did you receive a
> > notification about
> > https://github.com/apache/wicket/commit/4703c1ae1af91fdcca44e34edbeeb8bbaeb63b6c#commitcomment-7456068
> >  ?
> 
> I didn't get a notification. But I'll have a look.
> 
> Carl-Eric



Re: Add onAddToPage() to Component

2014-08-21 Thread Carl-Eric Menzel
On Wed, 20 Aug 2014 15:26:45 +0300
Martin Grigorov  wrote:

> Carl-Eric,
> 
> could you please re-format the related classes in wicket-6.x branch
> so all noise like
> https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677#diff-bfa8bce2a5e14af19e42011a5e2d4c68L4380
> disappear ?
> Just open the same classes in wicket-6.x branch, format them,
> commit+push. Cherry-pick in master.

Will do.

> I've added a comment on one of the commits in GitHub but so far I
> don't received a notification about it. Did you receive a
> notification about
> https://github.com/apache/wicket/commit/4703c1ae1af91fdcca44e34edbeeb8bbaeb63b6c#commitcomment-7456068
>  ?

I didn't get a notification. But I'll have a look.

Carl-Eric


Re: Add onAddToPage() to Component

2014-08-20 Thread Martin Grigorov
Carl-Eric,

could you please re-format the related classes in wicket-6.x branch so all
noise like
https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677#diff-bfa8bce2a5e14af19e42011a5e2d4c68L4380
disappear ?
Just open the same classes in wicket-6.x branch, format them, commit+push.
Cherry-pick in master.


I've added a comment on one of the commits in GitHub but so far I don't
received a notification about it. Did you receive a notification about
https://github.com/apache/wicket/commit/4703c1ae1af91fdcca44e34edbeeb8bbaeb63b6c#commitcomment-7456068
?


Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Wed, Aug 20, 2014 at 2:31 PM, Martin Grigorov 
wrote:

> to review:
> https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677
>
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
>
>
> On Mon, Aug 18, 2014 at 3:18 PM, Carl-Eric Menzel 
> wrote:
>
>> On Mon, 18 Aug 2014 13:30:03 +0200
>> Martin Grigorov  wrote:
>>
>> > I don't like the whole idea of re-adding component.
>> > Wizard is the only component in Wicket distro which fails
>> > the
>> > org.apache.wicket.core.util.objects.checker.OrphanComponentChecker. I
>> > think only the data (i.e. the models) should be kept around and the
>> > components should be re-created for every view of that data when this
>> > is needed. For me Wizard component is a bad practice.
>>
>> I'm not convinced it is entirely bad, but I can see that it is at least
>> not optimal.
>>
>> However, Wicket does not prohibit it and so I think we should make sure
>> it doesn't break in surprising ways.
>>
>> That said, I don't want to conflate this with the problems of Wizard
>> too much, because I still think we should have a complement to
>> onRemove, whether we're looking at Wizard or not, just to be consistent.
>>
>> I had a chat with Martin on IRC, he had a very good idea on how to
>> simplify onAddToPage to a more clearly defined onReAdd. Thank you
>> Martin!
>>
>> I've pushed an update including tests to the WICKET-5677 branch.
>>
>> The much simpler implementation is now not much more than an else branch
>> in Component#fireInitialize. If the component is already initialized,
>> it doesn't call onInitialize, but rather onReAdd. This way we have a
>> clear distinction between the two callbacks and we don't need a second
>> tree traversal, since this just uses the one that is there for
>> onInitialize already.
>>
>> Martin and everyone else, what do you think?
>>
>> Carl-Eric
>>
>
>


Re: Add onAddToPage() to Component

2014-08-20 Thread Martin Grigorov
to review: https://github.com/apache/wicket/compare/wicket-6.x...WICKET-5677

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Mon, Aug 18, 2014 at 3:18 PM, Carl-Eric Menzel 
wrote:

> On Mon, 18 Aug 2014 13:30:03 +0200
> Martin Grigorov  wrote:
>
> > I don't like the whole idea of re-adding component.
> > Wizard is the only component in Wicket distro which fails
> > the
> > org.apache.wicket.core.util.objects.checker.OrphanComponentChecker. I
> > think only the data (i.e. the models) should be kept around and the
> > components should be re-created for every view of that data when this
> > is needed. For me Wizard component is a bad practice.
>
> I'm not convinced it is entirely bad, but I can see that it is at least
> not optimal.
>
> However, Wicket does not prohibit it and so I think we should make sure
> it doesn't break in surprising ways.
>
> That said, I don't want to conflate this with the problems of Wizard
> too much, because I still think we should have a complement to
> onRemove, whether we're looking at Wizard or not, just to be consistent.
>
> I had a chat with Martin on IRC, he had a very good idea on how to
> simplify onAddToPage to a more clearly defined onReAdd. Thank you
> Martin!
>
> I've pushed an update including tests to the WICKET-5677 branch.
>
> The much simpler implementation is now not much more than an else branch
> in Component#fireInitialize. If the component is already initialized,
> it doesn't call onInitialize, but rather onReAdd. This way we have a
> clear distinction between the two callbacks and we don't need a second
> tree traversal, since this just uses the one that is there for
> onInitialize already.
>
> Martin and everyone else, what do you think?
>
> Carl-Eric
>


Re: Add onAddToPage() to Component

2014-08-18 Thread Carl-Eric Menzel
On Mon, 18 Aug 2014 13:30:03 +0200
Martin Grigorov  wrote:

> I don't like the whole idea of re-adding component.
> Wizard is the only component in Wicket distro which fails
> the
> org.apache.wicket.core.util.objects.checker.OrphanComponentChecker. I
> think only the data (i.e. the models) should be kept around and the
> components should be re-created for every view of that data when this
> is needed. For me Wizard component is a bad practice.

I'm not convinced it is entirely bad, but I can see that it is at least
not optimal.

However, Wicket does not prohibit it and so I think we should make sure
it doesn't break in surprising ways.

That said, I don't want to conflate this with the problems of Wizard
too much, because I still think we should have a complement to
onRemove, whether we're looking at Wizard or not, just to be consistent.

I had a chat with Martin on IRC, he had a very good idea on how to
simplify onAddToPage to a more clearly defined onReAdd. Thank you
Martin!

I've pushed an update including tests to the WICKET-5677 branch. 

The much simpler implementation is now not much more than an else branch
in Component#fireInitialize. If the component is already initialized,
it doesn't call onInitialize, but rather onReAdd. This way we have a
clear distinction between the two callbacks and we don't need a second
tree traversal, since this just uses the one that is there for
onInitialize already.

Martin and everyone else, what do you think?

Carl-Eric


Re: Add onAddToPage() to Component

2014-08-18 Thread Martin Grigorov
I don't like the whole idea of re-adding component.
Wizard is the only component in Wicket distro which fails
the org.apache.wicket.core.util.objects.checker.OrphanComponentChecker.
I think only the data (i.e. the models) should be kept around and the
components should be re-created for every view of that data when this is
needed.
For me Wizard component is a bad practice.

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Mon, Aug 18, 2014 at 1:07 PM, Carl-Eric Menzel 
wrote:

> One difficulty I am finding:
>
> If I do not simply fire onAdd for everything that gets added, then I
> have to differentiate. What do I do if I re-add a container that is
> already initialized, so its onReAdd should be called, but which has a
> new child that is not initialized? Which event should happen in the
> child, and how should it be propagated? Or the other way around, what
> if I have a new container but an already initialized child? If the
> container checks whether it is initialized before calling onReAdd on
> its children, then the child would not receive the onReAdd.
>
> This may look complicated, but I don't think it is a problem with the
> idea of having an onAdd or onReAdd. It is rather a consequence of the
> fact that Wicket allows components to be removed and added again in
> different compositions. You can remove a subtree and then add a new
> subtree that can have some of the old components.
>
> In light of this, I am in favor of having a general onAdd that is
> always called, and to more clearly document that onInitialize is
> supposed to be a delayed constructor.
>
> Carl-Eric
>
> On Mon, 18 Aug 2014 13:00:26 +0200
> Martin Grigorov  wrote:
>
> > Wait for some more opinions.
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> > https://twitter.com/mtgrigorov
> >
> >
> > On Mon, Aug 18, 2014 at 12:38 PM, Carl-Eric Menzel
> >  wrote:
> >
> > > On Mon, 18 Aug 2014 11:49:51 +0200
> > > Martin Grigorov  wrote:
> > >
> > > > Hi,
> > > >
> > > > I still find it confusing :-/
> > > >
> > > > 1) with this we will have: the constructor, onInitialize() and
> > > > onAddToPage() being called as component initializers. Way too much
> > > > IMO. onInitialize() and onAddToPage() are exactly the same thing
> > > > at this stage.
> > >
> > > At this stage, yes.
> > >
> > > > 2) onRemove() is not named onRemoveFromPage(). (I don't like the
> > > > name onAddToPage())
> > >
> > > I'm not invested in the name, so that can be changed :-)
> > >
> > > > Is it possible to redo it as onReAdd(), i.e. it will be called
> > > > when the component is added to a parent and it is already
> > > > initialized. Not sure but maybe all that is needed to do it is to
> > > > remove the call to onAddToPage() in onInitialize(), and rename
> > > > the method.
> > >
> > > Hm. That means that if I want to always react to being added, I'd
> > > have to do it in both onInitialize and onReAdd.
> > >
> > > However, I can see that it would potentially remove some confusion
> > > by more clearly defining the difference between the two.
> > >
> > > I'll give it a try and update the branch.
> > >
> > > Carl-Eric
> > >
>
>


Re: Add onAddToPage() to Component

2014-08-18 Thread Carl-Eric Menzel
One difficulty I am finding:

If I do not simply fire onAdd for everything that gets added, then I
have to differentiate. What do I do if I re-add a container that is
already initialized, so its onReAdd should be called, but which has a
new child that is not initialized? Which event should happen in the
child, and how should it be propagated? Or the other way around, what
if I have a new container but an already initialized child? If the
container checks whether it is initialized before calling onReAdd on
its children, then the child would not receive the onReAdd.

This may look complicated, but I don't think it is a problem with the
idea of having an onAdd or onReAdd. It is rather a consequence of the
fact that Wicket allows components to be removed and added again in
different compositions. You can remove a subtree and then add a new
subtree that can have some of the old components.

In light of this, I am in favor of having a general onAdd that is
always called, and to more clearly document that onInitialize is
supposed to be a delayed constructor.

Carl-Eric

On Mon, 18 Aug 2014 13:00:26 +0200
Martin Grigorov  wrote:

> Wait for some more opinions.
> 
> Martin Grigorov
> Wicket Training and Consulting
> https://twitter.com/mtgrigorov
> 
> 
> On Mon, Aug 18, 2014 at 12:38 PM, Carl-Eric Menzel
>  wrote:
> 
> > On Mon, 18 Aug 2014 11:49:51 +0200
> > Martin Grigorov  wrote:
> >
> > > Hi,
> > >
> > > I still find it confusing :-/
> > >
> > > 1) with this we will have: the constructor, onInitialize() and
> > > onAddToPage() being called as component initializers. Way too much
> > > IMO. onInitialize() and onAddToPage() are exactly the same thing
> > > at this stage.
> >
> > At this stage, yes.
> >
> > > 2) onRemove() is not named onRemoveFromPage(). (I don't like the
> > > name onAddToPage())
> >
> > I'm not invested in the name, so that can be changed :-)
> >
> > > Is it possible to redo it as onReAdd(), i.e. it will be called
> > > when the component is added to a parent and it is already
> > > initialized. Not sure but maybe all that is needed to do it is to
> > > remove the call to onAddToPage() in onInitialize(), and rename
> > > the method.
> >
> > Hm. That means that if I want to always react to being added, I'd
> > have to do it in both onInitialize and onReAdd.
> >
> > However, I can see that it would potentially remove some confusion
> > by more clearly defining the difference between the two.
> >
> > I'll give it a try and update the branch.
> >
> > Carl-Eric
> >



Re: Add onAddToPage() to Component

2014-08-18 Thread Martin Grigorov
Wait for some more opinions.

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Mon, Aug 18, 2014 at 12:38 PM, Carl-Eric Menzel 
wrote:

> On Mon, 18 Aug 2014 11:49:51 +0200
> Martin Grigorov  wrote:
>
> > Hi,
> >
> > I still find it confusing :-/
> >
> > 1) with this we will have: the constructor, onInitialize() and
> > onAddToPage() being called as component initializers. Way too much
> > IMO. onInitialize() and onAddToPage() are exactly the same thing at
> > this stage.
>
> At this stage, yes.
>
> > 2) onRemove() is not named onRemoveFromPage(). (I don't like the name
> > onAddToPage())
>
> I'm not invested in the name, so that can be changed :-)
>
> > Is it possible to redo it as onReAdd(), i.e. it will be called when
> > the component is added to a parent and it is already initialized. Not
> > sure but maybe all that is needed to do it is to remove the call to
> > onAddToPage() in onInitialize(), and rename the method.
>
> Hm. That means that if I want to always react to being added, I'd have
> to do it in both onInitialize and onReAdd.
>
> However, I can see that it would potentially remove some confusion by
> more clearly defining the difference between the two.
>
> I'll give it a try and update the branch.
>
> Carl-Eric
>


Re: Add onAddToPage() to Component

2014-08-18 Thread Carl-Eric Menzel
On Mon, 18 Aug 2014 11:49:51 +0200
Martin Grigorov  wrote:

> Hi,
> 
> I still find it confusing :-/
> 
> 1) with this we will have: the constructor, onInitialize() and
> onAddToPage() being called as component initializers. Way too much
> IMO. onInitialize() and onAddToPage() are exactly the same thing at
> this stage.

At this stage, yes. 

> 2) onRemove() is not named onRemoveFromPage(). (I don't like the name
> onAddToPage())

I'm not invested in the name, so that can be changed :-)

> Is it possible to redo it as onReAdd(), i.e. it will be called when
> the component is added to a parent and it is already initialized. Not
> sure but maybe all that is needed to do it is to remove the call to
> onAddToPage() in onInitialize(), and rename the method.

Hm. That means that if I want to always react to being added, I'd have
to do it in both onInitialize and onReAdd.

However, I can see that it would potentially remove some confusion by
more clearly defining the difference between the two.

I'll give it a try and update the branch.

Carl-Eric


Re: Add onAddToPage() to Component

2014-08-18 Thread Martin Grigorov
Hi,

I still find it confusing :-/

1) with this we will have: the constructor, onInitialize() and
onAddToPage() being called as component initializers. Way too much IMO.
onInitialize() and onAddToPage() are exactly the same thing at this stage.

2) onRemove() is not named onRemoveFromPage(). (I don't like the name
onAddToPage())

Is it possible to redo it as onReAdd(), i.e. it will be called when the
component is added to a parent and it is already initialized. Not sure but
maybe all that is needed to do it is to remove the call to onAddToPage() in
onInitialize(), and rename the method.

My 2c.


Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Mon, Aug 18, 2014 at 11:29 AM, Carl-Eric Menzel 
wrote:

> Hi everybody,
>
> I am currently dealing with
> https://issues.apache.org/jira/browse/WICKET-5265:
>
> A FencedFeedbackPanel marks the component it uses as a fence with a
> metadata entry, so that outer FencedFeedbackPanels do not traverse
> beyond that component to "steal" feedback messages. When an FFP is
> removed from the hierarchy (either by itself or as part of a subtree
> that is being removed), onRemove is called on it (and on all other
> components of that subtree). In onRemove, the FFP removes the fence
> mark. So far, so correct.
>
> When the FFP is added back to the page, there is no corresponding
> lifecycle callback that would tell it about the fact that it has been
> added back, so it cannot recreate the fence mark. This leads to outer
> FFPs stealing the messages.
>
> This "adding the same component back" is not a rare operation: It is
> for example what the Wizard component does with its WizardStep panels.
> They get removed and added all the time.
>
> I tried working around this by using onConfigure and/or onBeforeRender.
> In the really simple cases that works, but there can be more
> complicated component hierarchies. Even in a simple wizard the way
> Wicket traverses the tree and calls these two events can't guarantee
> that the inner FFP's onConfigure is called before the outer FFP tries
> to get the messages.
>
> I have uploaded a quickstart to the WICKET-5265 ticket to demonstrate
> this.
>
> Using a filter to prevent already rendered messages from being rendered
> again does not help, because then it is only the outer panel that shows
> them, not the inner one where they should be.
>
> martin-g suggested using onInitialize, but that does not work.
> onInitialize is called only once over the entire lifetime of the
> component, when it is first added to a page, and never again. It
> doesn't matter whether one sees it as basically a delayed constructor
> that allows calling overridden subclass methods, or as an indication of
> "hey, your page is here for the first time". The problem is that it is
> by definition only called once. The remove-and-add situation is not
> covered.
>
> onRemove is called every time a component (or a parent) is removed from
> the page hierarchy, so components can clean up after themselves.
>
> I think that just like behaviors have bind() and unbind(), components
> should have a kind of onAdd() to complement onRemove(). I have an
> implementation for this that is tested and works. With this, the
> problem described above can be elegantly solved.
>
> I named the method onAddToPage to make it more clear when it will be
> called and to reduce the chance for a name collision.
>
> I have defined it as follows:
> onAddToPage is called whenever a component is added to a parent in a
> way that connects it to the Page instance, but not before onInitialize
> has been called.
>
> For example:
>  - a component gets added to the page after it is constructed
>1. onInitialize
>2. onAddToPage
>
>  - a component is removed from the page and then added again
>1. onAddToPage
>
>  - a component is added to a container that is already added to the page
>1. onInitialize (if necessary)
>2. onAddToPage
>
>  - a component is added to a container that is *not yet* added to the
>page
>1. nothing is called!
>When the container is added to the page, its onAddToPage is called,
>and recursively for all its children, just like all the other
>lifecycle events.
>
> I think this is quite well-defined. martin-g commented there might be
> confusion with onInitialize.
>
> The javadoc of onInitialize says:
>
> > This method is meant to be used as an alternative to initialize
> > components. Usually the component's constructor is used for this task,
> > but sometimes a component cannot be initialized in isolation, it may
> > need to access its parent component or its markup in order to fully
> > initialize. This method is invoked once per component's lifecycle
> > when a path exists from this component to the {@link Page} thus
> > providing the component with an atomic callback
> > when the component's environment is built out.
>
> This is reasonably well-defined, though it completely ignores the fact
> that onRemove might hap

Add onAddToPage() to Component

2014-08-18 Thread Carl-Eric Menzel
Hi everybody,

I am currently dealing with
https://issues.apache.org/jira/browse/WICKET-5265:

A FencedFeedbackPanel marks the component it uses as a fence with a
metadata entry, so that outer FencedFeedbackPanels do not traverse
beyond that component to "steal" feedback messages. When an FFP is
removed from the hierarchy (either by itself or as part of a subtree
that is being removed), onRemove is called on it (and on all other
components of that subtree). In onRemove, the FFP removes the fence
mark. So far, so correct.

When the FFP is added back to the page, there is no corresponding
lifecycle callback that would tell it about the fact that it has been
added back, so it cannot recreate the fence mark. This leads to outer
FFPs stealing the messages.

This "adding the same component back" is not a rare operation: It is
for example what the Wizard component does with its WizardStep panels.
They get removed and added all the time.

I tried working around this by using onConfigure and/or onBeforeRender.
In the really simple cases that works, but there can be more
complicated component hierarchies. Even in a simple wizard the way
Wicket traverses the tree and calls these two events can't guarantee
that the inner FFP's onConfigure is called before the outer FFP tries
to get the messages.

I have uploaded a quickstart to the WICKET-5265 ticket to demonstrate
this.

Using a filter to prevent already rendered messages from being rendered
again does not help, because then it is only the outer panel that shows
them, not the inner one where they should be.

martin-g suggested using onInitialize, but that does not work.
onInitialize is called only once over the entire lifetime of the
component, when it is first added to a page, and never again. It
doesn't matter whether one sees it as basically a delayed constructor
that allows calling overridden subclass methods, or as an indication of
"hey, your page is here for the first time". The problem is that it is
by definition only called once. The remove-and-add situation is not
covered.

onRemove is called every time a component (or a parent) is removed from
the page hierarchy, so components can clean up after themselves.

I think that just like behaviors have bind() and unbind(), components
should have a kind of onAdd() to complement onRemove(). I have an
implementation for this that is tested and works. With this, the
problem described above can be elegantly solved.

I named the method onAddToPage to make it more clear when it will be
called and to reduce the chance for a name collision.

I have defined it as follows:
onAddToPage is called whenever a component is added to a parent in a
way that connects it to the Page instance, but not before onInitialize
has been called.

For example:
 - a component gets added to the page after it is constructed
   1. onInitialize
   2. onAddToPage

 - a component is removed from the page and then added again
   1. onAddToPage

 - a component is added to a container that is already added to the page
   1. onInitialize (if necessary)
   2. onAddToPage

 - a component is added to a container that is *not yet* added to the
   page
   1. nothing is called!
   When the container is added to the page, its onAddToPage is called,
   and recursively for all its children, just like all the other
   lifecycle events.

I think this is quite well-defined. martin-g commented there might be
confusion with onInitialize.

The javadoc of onInitialize says:

> This method is meant to be used as an alternative to initialize
> components. Usually the component's constructor is used for this task,
> but sometimes a component cannot be initialized in isolation, it may
> need to access its parent component or its markup in order to fully
> initialize. This method is invoked once per component's lifecycle 
> when a path exists from this component to the {@link Page} thus 
> providing the component with an atomic callback
> when the component's environment is built out.

This is reasonably well-defined, though it completely ignores the fact
that onRemove might happen. Also, it only says that onInitialize will
be called "sometime before Component#onBeforeRender()" which I think is
more confusing than anything else. It should probably be changed to
"just before onConfigure".

I propose this for documentation of onAddToPage:

/**
 * This method is called whenever a component is added to the component
 * tree connected to the page,
 * to allow it to initialize things that depend on other
 * components in the page. This is similar
 * to {@link #onInitialize()}. However, onInitialize is only
 * ever called once, the first time the
 * component is added. It is not called when a component is
 * removed and then added again.
 *
 * This method, however, is called every time the component is
 * added to the page's tree. It is
 * never called before onInitialize(). It is thus the opposite
 * of onRemove().
 *
 * Subclasses that override this must call super.onAddToPage().