On 26/10/2015 13:11, Martin Grigorov 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
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 <[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.
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 :-)

Reply via email to