You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2015/10/26 13:11:58 UTC

Re: wicket git commit: improved WICKET-5988 relaying on component resolvers

Hi Andrea,

On Mon, Oct 26, 2015 at 2:02 PM, <ad...@apache.org> wrote:

> Repository: wicket
> Updated Branches:
>   refs/heads/wicket-7.x 92e9d6dd1 -> f0364dad0
>
>
> improved WICKET-5988 relaying on component resolvers
>

Please do not make changes to already released tickets. WICKET-5988 is part
of 7.1.0.
Better create a new ticket and explain what is the new problem and what is
the improvement


>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/f0364dad
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/f0364dad
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/f0364dad
>
> Branch: refs/heads/wicket-7.x
> Commit: f0364dad0db9d1d3b49891ca9a49662a4e69e378
> Parents: 92e9d6d
> Author: Andrea Del Bene <ad...@apache.org>
> Authored: Mon Oct 26 13:00:53 2015 +0100
> Committer: Andrea Del Bene <ad...@apache.org>
> Committed: Mon Oct 26 13:02:27 2015 +0100
>
> ----------------------------------------------------------------------
>  .../wicket/markup/html/border/Border.java       | 27 +++++++++-----------
>  .../form/feedback/FeedbackFormPage_result2.html |  2 +-
>  2 files changed, 13 insertions(+), 16 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
> index 766d296..f0d5412 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
> @@ -166,7 +166,7 @@ public abstract class Border extends
> WebMarkupContainer implements IComponentRes
>                 super(id, model);
>
>                 body = new BorderBodyContainer(id + "_" + BODY);
> -               addToBorder(body);
> +               //addToBorder(body);
>

This line should be removed, not just commented out.


>         }
>
>         /**
> @@ -205,7 +205,17 @@ public abstract class Border extends
> WebMarkupContainer implements IComponentRes
>         @Override
>         public Border add(final Component... children)
>         {
> -               getBodyContainer().add(children);
> +               for (Component component : children)
> +               {
> +                       if (component.equals(body))
>

remove() uses identity check:
https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java#L222


> +                       {
> +                               addToBorder(component);
> +                       }
> +                       else
> +                       {
> +                               getBodyContainer().add(component);
> +                       }
> +               }
>

This change looks wrong.
The contract is:
 - add() adds to the border's body.
 - addToBorder() adds to the border itself

All other methods like: replace(), remove(), addOrReplace(), etc. should
behave as add(), i.e. they should manipulate the border's body.


>                 return this;
>         }
>
> @@ -631,17 +641,4 @@ public abstract class Border extends
> WebMarkupContainer implements IComponentRes
>                 return borderMarkup != null ? borderMarkup : markup;
>         }
>
> -       @Override
> -       protected void onBeforeRender()
> -       {
> -               super.onBeforeRender();
> -               /**
> -                * https://issues.apache.org/jira/browse/WICKET-5981
> -                * dequeue border to adjust children hierarchy.
> -                */
> -               if (!hasBeenRendered())
> -               {
> -                       dequeue();
> -               }
> -       }
>  }
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
> index da7d6b2..ced751f 100644
> ---
> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
> +++
> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
> @@ -1,6 +1,6 @@
>  <html>
>  <body>
> -<form wicket:id="form" id="form1" method="post"
> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?1-2.IFormSubmitListener-form"><div
> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
> +<form wicket:id="form" id="form1" method="post"
> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?0-2.IFormSubmitListener-form"><div
> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
>  <span wicket:id="feedback"><wicket:border>
>                 <wicket:body>
>         <input type="text" wicket:id="input" value=""
> name="feedback:feedback_body:input">
>
>

IMO the previous version of the fix was better.

Re: wicket git commit: improved WICKET-5988 relaying on component resolvers

Posted by andrea del bene <an...@gmail.com>.

On 27/10/2015 15:40, Martin Grigorov wrote:
> On Tue, Oct 27, 2015 at 4:25 PM, andrea del bene <an...@gmail.com>
> wrote:
>
>>
>> On 26/10/2015 13:11, Martin Grigorov wrote:
>>
>>> Hi Andrea,
>>>
>>> On Mon, Oct 26, 2015 at 2:02 PM, <ad...@apache.org> wrote:
>>>
>>> Repository: wicket
>>>> Updated Branches:
>>>>     refs/heads/wicket-7.x 92e9d6dd1 -> f0364dad0
>>>>
>>>>
>>>> improved WICKET-5988 relaying on component resolvers
>>>>
>>>> Please do not make changes to already released tickets. WICKET-5988 is
>>> part
>>> of 7.1.0.
>>> Better create a new ticket and explain what is the new problem and what is
>>> the improvement
>>>
>> The rational behind this change is that the previous fix was more a
>> workaround for WICKET-5988 while this new one is based on something that is
>> already in place, that is the component-resolving capability of Border.
>
> It doesn't matter.
> Since WICKET-5988 is closed and part of a release already new commits
> should use a new ticket.
> If a user finds a regression in 7.2.0 (and there is a regression! see
> below) then it is impossible to see a related ticket in the changelog
>
My fault. I said why I did it, but I also agree that I should track it 
in a new issue.
>
>
> Well, it was more clear to me what happens.
> If the new version is fixed and some documentation (javadoc) is added then
> I'll be happier.
No problem. I will add some JavaDoc and change the code as you suggested.


Re: wicket git commit: improved WICKET-5988 relaying on component resolvers

Posted by Martin Grigorov <mg...@apache.org>.
On Tue, Oct 27, 2015 at 4:25 PM, andrea del bene <an...@gmail.com>
wrote:

>
>
> On 26/10/2015 13:11, Martin Grigorov wrote:
>
>> Hi Andrea,
>>
>> On Mon, Oct 26, 2015 at 2:02 PM, <ad...@apache.org> wrote:
>>
>> Repository: wicket
>>> Updated Branches:
>>>    refs/heads/wicket-7.x 92e9d6dd1 -> f0364dad0
>>>
>>>
>>> improved WICKET-5988 relaying on component resolvers
>>>
>>> Please do not make changes to already released tickets. WICKET-5988 is
>> part
>> of 7.1.0.
>> Better create a new ticket and explain what is the new problem and what is
>> the improvement
>>
> The rational behind this change is that the previous fix was more a
> workaround for WICKET-5988 while this new one is based on something that is
> already in place, that is the component-resolving capability of Border.


It doesn't matter.
Since WICKET-5988 is closed and part of a release already new commits
should use a new ticket.
If a user finds a regression in 7.2.0 (and there is a regression! see
below) then it is impossible to see a related ticket in the changelog


>
>
>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/f0364dad
>>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/f0364dad
>>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/f0364dad
>>>
>>> Branch: refs/heads/wicket-7.x
>>> Commit: f0364dad0db9d1d3b49891ca9a49662a4e69e378
>>> Parents: 92e9d6d
>>> Author: Andrea Del Bene <ad...@apache.org>
>>> Authored: Mon Oct 26 13:00:53 2015 +0100
>>> Committer: Andrea Del Bene <ad...@apache.org>
>>> Committed: Mon Oct 26 13:02:27 2015 +0100
>>>
>>> ----------------------------------------------------------------------
>>>   .../wicket/markup/html/border/Border.java       | 27
>>> +++++++++-----------
>>>   .../form/feedback/FeedbackFormPage_result2.html |  2 +-
>>>   2 files changed, 13 insertions(+), 16 deletions(-)
>>> ----------------------------------------------------------------------
>>>
>>>
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>>> ----------------------------------------------------------------------
>>> diff --git
>>>
>>> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>>>
>>> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>>> index 766d296..f0d5412 100644
>>> ---
>>>
>>> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>>> +++
>>>
>>> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>>> @@ -166,7 +166,7 @@ public abstract class Border extends
>>> WebMarkupContainer implements IComponentRes
>>>                  super(id, model);
>>>
>>>                  body = new BorderBodyContainer(id + "_" + BODY);
>>> -               addToBorder(body);
>>> +               //addToBorder(body);
>>>
>>> This line should be removed, not just commented out.
>>
> right
>
>>
>>
>>          }
>>>
>>>          /**
>>> @@ -205,7 +205,17 @@ public abstract class Border extends
>>> WebMarkupContainer implements IComponentRes
>>>          @Override
>>>          public Border add(final Component... children)
>>>          {
>>> -               getBodyContainer().add(children);
>>> +               for (Component component : children)
>>> +               {
>>> +                       if (component.equals(body))
>>>
>>> remove() uses identity check:
>>
>> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java#L222
>>
>> agree. I should be coherent with the existing code.
>
>> +                       {
>>> +                               addToBorder(component);
>>> +                       }
>>> +                       else
>>> +                       {
>>> +                               getBodyContainer().add(component);
>>> +                       }
>>> +               }
>>>
>>> This change looks wrong.
>> The contract is:
>>   - add() adds to the border's body.
>>   - addToBorder() adds to the border itself
>>
>> All other methods like: replace(), remove(), addOrReplace(), etc. should
>> behave as add(), i.e. they should manipulate the border's body.
>>
> I don't see the problem. remove() has exactly the same behaviour. What's
> wrong if we handle body component in a different way?


The problem I see is that now addOrReplace() doesn't behave as #add(). Nor
#replace(), nor #remove(String). All of them should have the same checks
inside.
Border.java is the main reason why #add(), #replace() and others are no
more 'final' methods in MarkupContainer.
If Wicket-core cannot make sure that it overrides them consistently then we
cannot expect that the applications will do it properly.


>
>
>>                  return this;
>>>          }
>>>
>>> @@ -631,17 +641,4 @@ public abstract class Border extends
>>> WebMarkupContainer implements IComponentRes
>>>                  return borderMarkup != null ? borderMarkup : markup;
>>>          }
>>>
>>> -       @Override
>>> -       protected void onBeforeRender()
>>> -       {
>>> -               super.onBeforeRender();
>>> -               /**
>>> -                * https://issues.apache.org/jira/browse/WICKET-5981
>>> -                * dequeue border to adjust children hierarchy.
>>> -                */
>>> -               if (!hasBeenRendered())
>>> -               {
>>> -                       dequeue();
>>> -               }
>>> -       }
>>>   }
>>>
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>>> ----------------------------------------------------------------------
>>> diff --git
>>>
>>> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>>>
>>> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>>> index da7d6b2..ced751f 100644
>>> ---
>>>
>>> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>>> +++
>>>
>>> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>>> @@ -1,6 +1,6 @@
>>>   <html>
>>>   <body>
>>> -<form wicket:id="form" id="form1" method="post"
>>>
>>> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?1-2.IFormSubmitListener-form"><div
>>>
>>> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
>>> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
>>> +<form wicket:id="form" id="form1" method="post"
>>>
>>> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?0-2.IFormSubmitListener-form"><div
>>>
>>> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
>>> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
>>>   <span wicket:id="feedback"><wicket:border>
>>>                  <wicket:body>
>>>          <input type="text" wicket:id="input" value=""
>>> name="feedback:feedback_body:input">
>>>
>>>
>>> IMO the previous version of the fix was better.
>>
> As I said, the previous solution was more a workaround rather than a real
> solution. But that's my opinion :-)
>
>
Well, it was more clear to me what happens.
If the new version is fixed and some documentation (javadoc) is added then
I'll be happier.

Re: wicket git commit: improved WICKET-5988 relaying on component resolvers

Posted by andrea del bene <an...@gmail.com>.

On 26/10/2015 13:11, Martin Grigorov wrote:
> Hi Andrea,
>
> On Mon, Oct 26, 2015 at 2:02 PM, <ad...@apache.org> wrote:
>
>> Repository: wicket
>> Updated Branches:
>>    refs/heads/wicket-7.x 92e9d6dd1 -> f0364dad0
>>
>>
>> improved WICKET-5988 relaying on component resolvers
>>
> Please do not make changes to already released tickets. WICKET-5988 is part
> of 7.1.0.
> Better create a new ticket and explain what is the new problem and what is
> the improvement
The rational behind this change is that the previous fix was more a 
workaround for WICKET-5988 while this new one is based on something that 
is already in place, that is the component-resolving capability of Border.

>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/f0364dad
>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/f0364dad
>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/f0364dad
>>
>> Branch: refs/heads/wicket-7.x
>> Commit: f0364dad0db9d1d3b49891ca9a49662a4e69e378
>> Parents: 92e9d6d
>> Author: Andrea Del Bene <ad...@apache.org>
>> Authored: Mon Oct 26 13:00:53 2015 +0100
>> Committer: Andrea Del Bene <ad...@apache.org>
>> Committed: Mon Oct 26 13:02:27 2015 +0100
>>
>> ----------------------------------------------------------------------
>>   .../wicket/markup/html/border/Border.java       | 27 +++++++++-----------
>>   .../form/feedback/FeedbackFormPage_result2.html |  2 +-
>>   2 files changed, 13 insertions(+), 16 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> index 766d296..f0d5412 100644
>> ---
>> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> +++
>> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> @@ -166,7 +166,7 @@ public abstract class Border extends
>> WebMarkupContainer implements IComponentRes
>>                  super(id, model);
>>
>>                  body = new BorderBodyContainer(id + "_" + BODY);
>> -               addToBorder(body);
>> +               //addToBorder(body);
>>
> This line should be removed, not just commented out.
right
>
>
>>          }
>>
>>          /**
>> @@ -205,7 +205,17 @@ public abstract class Border extends
>> WebMarkupContainer implements IComponentRes
>>          @Override
>>          public Border add(final Component... children)
>>          {
>> -               getBodyContainer().add(children);
>> +               for (Component component : children)
>> +               {
>> +                       if (component.equals(body))
>>
> remove() uses identity check:
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java#L222
>
agree. I should be coherent with the existing code.
>> +                       {
>> +                               addToBorder(component);
>> +                       }
>> +                       else
>> +                       {
>> +                               getBodyContainer().add(component);
>> +                       }
>> +               }
>>
> This change looks wrong.
> The contract is:
>   - add() adds to the border's body.
>   - addToBorder() adds to the border itself
>
> All other methods like: replace(), remove(), addOrReplace(), etc. should
> behave as add(), i.e. they should manipulate the border's body.
I don't see the problem. remove() has exactly the same behaviour. What's 
wrong if we handle body component in a different way?
>
>>                  return this;
>>          }
>>
>> @@ -631,17 +641,4 @@ public abstract class Border extends
>> WebMarkupContainer implements IComponentRes
>>                  return borderMarkup != null ? borderMarkup : markup;
>>          }
>>
>> -       @Override
>> -       protected void onBeforeRender()
>> -       {
>> -               super.onBeforeRender();
>> -               /**
>> -                * https://issues.apache.org/jira/browse/WICKET-5981
>> -                * dequeue border to adjust children hierarchy.
>> -                */
>> -               if (!hasBeenRendered())
>> -               {
>> -                       dequeue();
>> -               }
>> -       }
>>   }
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> index da7d6b2..ced751f 100644
>> ---
>> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> +++
>> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> @@ -1,6 +1,6 @@
>>   <html>
>>   <body>
>> -<form wicket:id="form" id="form1" method="post"
>> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?1-2.IFormSubmitListener-form"><div
>> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
>> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
>> +<form wicket:id="form" id="form1" method="post"
>> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?0-2.IFormSubmitListener-form"><div
>> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
>> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
>>   <span wicket:id="feedback"><wicket:border>
>>                  <wicket:body>
>>          <input type="text" wicket:id="input" value=""
>> name="feedback:feedback_body:input">
>>
>>
> IMO the previous version of the fix was better.
As I said, the previous solution was more a workaround rather than a 
real solution. But that's my opinion :-)


Re: wicket git commit: improved WICKET-5988 relaying on component resolvers

Posted by Martin Grigorov <mg...@apache.org>.
Any other opinions on the change ?
I see the change have been applied to master too.

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

On Mon, Oct 26, 2015 at 2:11 PM, Martin Grigorov <mg...@apache.org>
wrote:

> Hi Andrea,
>
> On Mon, Oct 26, 2015 at 2:02 PM, <ad...@apache.org> wrote:
>
>> Repository: wicket
>> Updated Branches:
>>   refs/heads/wicket-7.x 92e9d6dd1 -> f0364dad0
>>
>>
>> improved WICKET-5988 relaying on component resolvers
>>
>
> Please do not make changes to already released tickets. WICKET-5988 is
> part of 7.1.0.
> Better create a new ticket and explain what is the new problem and what is
> the improvement
>
>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/f0364dad
>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/f0364dad
>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/f0364dad
>>
>> Branch: refs/heads/wicket-7.x
>> Commit: f0364dad0db9d1d3b49891ca9a49662a4e69e378
>> Parents: 92e9d6d
>> Author: Andrea Del Bene <ad...@apache.org>
>> Authored: Mon Oct 26 13:00:53 2015 +0100
>> Committer: Andrea Del Bene <ad...@apache.org>
>> Committed: Mon Oct 26 13:02:27 2015 +0100
>>
>> ----------------------------------------------------------------------
>>  .../wicket/markup/html/border/Border.java       | 27 +++++++++-----------
>>  .../form/feedback/FeedbackFormPage_result2.html |  2 +-
>>  2 files changed, 13 insertions(+), 16 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> index 766d296..f0d5412 100644
>> ---
>> a/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> +++
>> b/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java
>> @@ -166,7 +166,7 @@ public abstract class Border extends
>> WebMarkupContainer implements IComponentRes
>>                 super(id, model);
>>
>>                 body = new BorderBodyContainer(id + "_" + BODY);
>> -               addToBorder(body);
>> +               //addToBorder(body);
>>
>
> This line should be removed, not just commented out.
>
>
>>         }
>>
>>         /**
>> @@ -205,7 +205,17 @@ public abstract class Border extends
>> WebMarkupContainer implements IComponentRes
>>         @Override
>>         public Border add(final Component... children)
>>         {
>> -               getBodyContainer().add(children);
>> +               for (Component component : children)
>> +               {
>> +                       if (component.equals(body))
>>
>
> remove() uses identity check:
>
> https://github.com/apache/wicket/blob/master/wicket-core/src/main/java/org/apache/wicket/markup/html/border/Border.java#L222
>
>
>> +                       {
>> +                               addToBorder(component);
>> +                       }
>> +                       else
>> +                       {
>> +                               getBodyContainer().add(component);
>> +                       }
>> +               }
>>
>
> This change looks wrong.
> The contract is:
>  - add() adds to the border's body.
>  - addToBorder() adds to the border itself
>
> All other methods like: replace(), remove(), addOrReplace(), etc. should
> behave as add(), i.e. they should manipulate the border's body.
>
>
>>                 return this;
>>         }
>>
>> @@ -631,17 +641,4 @@ public abstract class Border extends
>> WebMarkupContainer implements IComponentRes
>>                 return borderMarkup != null ? borderMarkup : markup;
>>         }
>>
>> -       @Override
>> -       protected void onBeforeRender()
>> -       {
>> -               super.onBeforeRender();
>> -               /**
>> -                * https://issues.apache.org/jira/browse/WICKET-5981
>> -                * dequeue border to adjust children hierarchy.
>> -                */
>> -               if (!hasBeenRendered())
>> -               {
>> -                       dequeue();
>> -               }
>> -       }
>>  }
>>
>>
>> http://git-wip-us.apache.org/repos/asf/wicket/blob/f0364dad/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> ----------------------------------------------------------------------
>> diff --git
>> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> index da7d6b2..ced751f 100644
>> ---
>> a/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> +++
>> b/wicket-core/src/test/java/org/apache/wicket/markup/html/form/feedback/FeedbackFormPage_result2.html
>> @@ -1,6 +1,6 @@
>>  <html>
>>  <body>
>> -<form wicket:id="form" id="form1" method="post"
>> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?1-2.IFormSubmitListener-form"><div
>> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
>> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
>> +<form wicket:id="form" id="form1" method="post"
>> action="./org.apache.wicket.markup.html.form.feedback.FeedbackFormPage?0-2.IFormSubmitListener-form"><div
>> style="width:0px;height:0px;position:absolute;left:-100px;top:-100px;overflow:hidden"><input
>> type="hidden" name="form1_hf_0" id="form1_hf_0" /></div>
>>  <span wicket:id="feedback"><wicket:border>
>>                 <wicket:body>
>>         <input type="text" wicket:id="input" value=""
>> name="feedback:feedback_body:input">
>>
>>
>
> IMO the previous version of the fix was better.
>
>