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 <[email protected]>
wrote:

> Hi Andrea,
>
> On Mon, Oct 26, 2015 at 2:02 PM, <[email protected]> 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 <[email protected]>
>> Authored: Mon Oct 26 13:00:53 2015 +0100
>> Committer: Andrea Del Bene <[email protected]>
>> 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.
>
>

Reply via email to