You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by pa...@apache.org on 2020/01/28 20:19:16 UTC

[wicket] branch csp updated: WICKET-6725: fixed special Form placeholder rendering

This is an automated email from the ASF dual-hosted git repository.

papegaaij pushed a commit to branch csp
in repository https://gitbox.apache.org/repos/asf/wicket.git


The following commit(s) were added to refs/heads/csp by this push:
     new fb290e8  WICKET-6725: fixed special Form placeholder rendering
fb290e8 is described below

commit fb290e818a278eb69f4fa64d77514d8f3ee3068c
Author: Emond Papegaaij <em...@topicus.nl>
AuthorDate: Tue Jan 28 21:18:50 2020 +0100

    WICKET-6725: fixed special Form placeholder rendering
---
 .../main/java/org/apache/wicket/markup/html/form/Form.java    | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
index a39160a..2ffc52b 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
@@ -1698,14 +1698,9 @@ public class Form<T> extends WebMarkupContainer
 		else
 		{
 			// rewrite inner form tag as div
-			response.write("<div style=\"display:none\"");
-			if (getOutputMarkupId())
-			{
-				response.write(" id=\"");
-				response.write(getMarkupId());
-				response.write("\"");
-			}
-			response.write("></div>");
+			response.write(
+				String.format("<div id=\"%s\" class=\"%s\" data-wicket-placeholder=\"\"></div>",
+					getAjaxRegionMarkupId(), getString(CssUtils.key(Component.class, "hidden"))));
 		}
 	}
 


Re: [wicket] branch csp updated: WICKET-6725: fixed special Form placeholder rendering

Posted by Maxim Solodovnik <so...@gmail.com>.
Hello Martin,

please see the discussion here:
https://github.com/apache/wicket/commit/e1bdfe74cc7d3c0fdb67712a665cb15164868bc1
 :))

On Wed, 29 Jan 2020 at 15:12, Martin Grigorov <mg...@apache.org> wrote:

> On Tue, Jan 28, 2020 at 10:19 PM <pa...@apache.org> wrote:
>
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > papegaaij pushed a commit to branch csp
> > in repository https://gitbox.apache.org/repos/asf/wicket.git
> >
> >
> > The following commit(s) were added to refs/heads/csp by this push:
> >      new fb290e8  WICKET-6725: fixed special Form placeholder rendering
> > fb290e8 is described below
> >
> > commit fb290e818a278eb69f4fa64d77514d8f3ee3068c
> > Author: Emond Papegaaij <em...@topicus.nl>
> > AuthorDate: Tue Jan 28 21:18:50 2020 +0100
> >
> >     WICKET-6725: fixed special Form placeholder rendering
> > ---
> >  .../main/java/org/apache/wicket/markup/html/form/Form.java    | 11
> > +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git
> > a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > index a39160a..2ffc52b 100644
> > ---
> > a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > +++
> > b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > @@ -1698,14 +1698,9 @@ public class Form<T> extends WebMarkupContainer
> >                 else
> >                 {
> >                         // rewrite inner form tag as div
> > -                       response.write("<div style=\"display:none\"");
> > -                       if (getOutputMarkupId())
> > -                       {
> > -                               response.write(" id=\"");
> > -                               response.write(getMarkupId());
> > -                               response.write("\"");
> > -                       }
> > -                       response.write("></div>");
> > +                       response.write(
> > +                               String.format("<div id=\"%s\"
> class=\"%s\"
> > data-wicket-placeholder=\"\"></div>",
> > +                                       getAjaxRegionMarkupId(),
> > getString(CssUtils.key(Component.class, "hidden"))));
> >
>
> Why did you replace the usage of getOutputMarkupId()+getMarkupId()
> with getAjaxRegionMarkupId() ?
> Please use better commit messages! It is not clear whether you wanted to
> actually make this change or broke it unintentionally.
>
>
> >                 }
> >         }
> >
> >
> >
>


-- 
WBR
Maxim aka solomax

Re: [wicket] branch csp updated: WICKET-6725: fixed special Form placeholder rendering

Posted by Emond Papegaaij <em...@gmail.com>.
Hi Martin,

You are right, the commit message could have used a bit more
explanation. As you can see at Github, the change was intentional.
Unfortunately there seems to be no testcase covering this piece of
code, but IMHO it was broken and is fixed now. From what I can see,
the only difference between the method in Form and in Component should
be that the tag name must be changed to div when the form is nested.
The check for getOutputMarkupId is bogus, a placeholder always has a
markup id. The code for getAjaxRegionMarkupId seemed appropriate. The
code was also missing the data attribute. I assumed we simply forgot
to keep this method up to date while we changed its
super-implementation. Please correct me if I'm wrong.

Emond

On Wed, Jan 29, 2020 at 9:12 AM Martin Grigorov <mg...@apache.org> wrote:
>
> On Tue, Jan 28, 2020 at 10:19 PM <pa...@apache.org> wrote:
>
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > papegaaij pushed a commit to branch csp
> > in repository https://gitbox.apache.org/repos/asf/wicket.git
> >
> >
> > The following commit(s) were added to refs/heads/csp by this push:
> >      new fb290e8  WICKET-6725: fixed special Form placeholder rendering
> > fb290e8 is described below
> >
> > commit fb290e818a278eb69f4fa64d77514d8f3ee3068c
> > Author: Emond Papegaaij <em...@topicus.nl>
> > AuthorDate: Tue Jan 28 21:18:50 2020 +0100
> >
> >     WICKET-6725: fixed special Form placeholder rendering
> > ---
> >  .../main/java/org/apache/wicket/markup/html/form/Form.java    | 11
> > +++--------
> >  1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git
> > a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > index a39160a..2ffc52b 100644
> > ---
> > a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > +++
> > b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> > @@ -1698,14 +1698,9 @@ public class Form<T> extends WebMarkupContainer
> >                 else
> >                 {
> >                         // rewrite inner form tag as div
> > -                       response.write("<div style=\"display:none\"");
> > -                       if (getOutputMarkupId())
> > -                       {
> > -                               response.write(" id=\"");
> > -                               response.write(getMarkupId());
> > -                               response.write("\"");
> > -                       }
> > -                       response.write("></div>");
> > +                       response.write(
> > +                               String.format("<div id=\"%s\" class=\"%s\"
> > data-wicket-placeholder=\"\"></div>",
> > +                                       getAjaxRegionMarkupId(),
> > getString(CssUtils.key(Component.class, "hidden"))));
> >
>
> Why did you replace the usage of getOutputMarkupId()+getMarkupId()
> with getAjaxRegionMarkupId() ?
> Please use better commit messages! It is not clear whether you wanted to
> actually make this change or broke it unintentionally.
>
>
> >                 }
> >         }
> >
> >
> >

Re: [wicket] branch csp updated: WICKET-6725: fixed special Form placeholder rendering

Posted by Martin Grigorov <mg...@apache.org>.
On Tue, Jan 28, 2020 at 10:19 PM <pa...@apache.org> wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> papegaaij pushed a commit to branch csp
> in repository https://gitbox.apache.org/repos/asf/wicket.git
>
>
> The following commit(s) were added to refs/heads/csp by this push:
>      new fb290e8  WICKET-6725: fixed special Form placeholder rendering
> fb290e8 is described below
>
> commit fb290e818a278eb69f4fa64d77514d8f3ee3068c
> Author: Emond Papegaaij <em...@topicus.nl>
> AuthorDate: Tue Jan 28 21:18:50 2020 +0100
>
>     WICKET-6725: fixed special Form placeholder rendering
> ---
>  .../main/java/org/apache/wicket/markup/html/form/Form.java    | 11
> +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> index a39160a..2ffc52b 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> @@ -1698,14 +1698,9 @@ public class Form<T> extends WebMarkupContainer
>                 else
>                 {
>                         // rewrite inner form tag as div
> -                       response.write("<div style=\"display:none\"");
> -                       if (getOutputMarkupId())
> -                       {
> -                               response.write(" id=\"");
> -                               response.write(getMarkupId());
> -                               response.write("\"");
> -                       }
> -                       response.write("></div>");
> +                       response.write(
> +                               String.format("<div id=\"%s\" class=\"%s\"
> data-wicket-placeholder=\"\"></div>",
> +                                       getAjaxRegionMarkupId(),
> getString(CssUtils.key(Component.class, "hidden"))));
>

Why did you replace the usage of getOutputMarkupId()+getMarkupId()
with getAjaxRegionMarkupId() ?
Please use better commit messages! It is not clear whether you wanted to
actually make this change or broke it unintentionally.


>                 }
>         }
>
>
>

Re: [wicket] branch csp updated: WICKET-6725: fixed special Form placeholder rendering

Posted by Martin Grigorov <mg...@apache.org>.
On Tue, Jan 28, 2020 at 10:19 PM <pa...@apache.org> wrote:

> This is an automated email from the ASF dual-hosted git repository.
>
> papegaaij pushed a commit to branch csp
> in repository https://gitbox.apache.org/repos/asf/wicket.git
>
>
> The following commit(s) were added to refs/heads/csp by this push:
>      new fb290e8  WICKET-6725: fixed special Form placeholder rendering
> fb290e8 is described below
>
> commit fb290e818a278eb69f4fa64d77514d8f3ee3068c
> Author: Emond Papegaaij <em...@topicus.nl>
> AuthorDate: Tue Jan 28 21:18:50 2020 +0100
>
>     WICKET-6725: fixed special Form placeholder rendering
> ---
>  .../main/java/org/apache/wicket/markup/html/form/Form.java    | 11
> +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> index a39160a..2ffc52b 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java
> @@ -1698,14 +1698,9 @@ public class Form<T> extends WebMarkupContainer
>                 else
>                 {
>                         // rewrite inner form tag as div
> -                       response.write("<div style=\"display:none\"");
> -                       if (getOutputMarkupId())
> -                       {
> -                               response.write(" id=\"");
> -                               response.write(getMarkupId());
> -                               response.write("\"");
> -                       }
> -                       response.write("></div>");
> +                       response.write(
> +                               String.format("<div id=\"%s\" class=\"%s\"
> data-wicket-placeholder=\"\"></div>",
> +                                       getAjaxRegionMarkupId(),
> getString(CssUtils.key(Component.class, "hidden"))));
>

Why did you replace the usage of getOutputMarkupId()+getMarkupId()
with getAjaxRegionMarkupId() ?
Please use better commit messages! It is not clear whether you wanted to
actually make this change or broke it unintentionally.


>                 }
>         }
>
>
>