Hi Andrea,

On Wed, Oct 15, 2014 at 1:33 PM, <[email protected]> wrote:

> Repository: wicket
> Updated Branches:
>   refs/heads/master 46684ffdf -> 8953bd2cb
>
>
> WICKET-5724 Queueing component in autocomponent
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/8953bd2c
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/8953bd2c
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/8953bd2c
>
> Branch: refs/heads/master
> Commit: 8953bd2cb91f8b62d4f26395156986b19e161720
> Parents: 46684ff
> Author: adelbene <[email protected]>
> Authored: Wed Oct 15 12:14:45 2014 +0200
> Committer: adelbene <[email protected]>
> Committed: Wed Oct 15 12:33:32 2014 +0200
>
> ----------------------------------------------------------------------
>  .../java/org/apache/wicket/MarkupContainer.java | 71 ++++++++++++++------
>  .../filter/RelativePathPrefixHandler.java       | 59 +++-------------
>  2 files changed, 61 insertions(+), 69 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/8953bd2c/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
> b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
> index 1ec4f02..4b40921 100644
> --- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
> +++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java
> @@ -35,7 +35,6 @@ import org.apache.wicket.markup.MarkupStream;
>  import org.apache.wicket.markup.MarkupType;
>  import org.apache.wicket.markup.WicketTag;
>  import org.apache.wicket.markup.html.border.Border;
> -import org.apache.wicket.markup.html.internal.InlineEnclosure;
>  import org.apache.wicket.markup.resolver.ComponentResolvers;
>  import org.apache.wicket.model.IComponentInheritedModel;
>  import org.apache.wicket.model.IModel;
> @@ -262,9 +261,31 @@ public abstract class MarkupContainer extends
> Component implements Iterable<Comp
>                         children_remove(index);
>                 }
>                 add(component);
> -
> +
> +               /**
> +                * https://issues.apache.org/jira/browse/WICKET-5724
> +                */
> +               queueChildContainer(component);
> +
>                 return true;
>         }
> +
> +       /**
> +        * Try to queue a child markup container if the current
> +        * component queue still contain items.
>

Please add more info - in what circumstances this is needed ?


> +        *
> +        * @param component
> +        *      The children markup container
> +        */
> +       private void queueChildContainer(final Component component)
> +       {
> +               if (queue != null && !queue.isEmpty() &&
> +                       component instanceof MarkupContainer)
> +               {
> +                       MarkupContainer childContainer =
> (MarkupContainer)component;
> +                       childContainer.queue(queue);
> +               }
> +       }
>
>         /**
>          * @param component
> @@ -1661,24 +1682,8 @@ public abstract class MarkupContainer extends
> Component implements Iterable<Comp
>                         {
>                                 Component component = (Component)child;
>                                 component.detach();
> -
> -                               // We need to keep InlineEnclosures for
> Ajax request handling.
> -                               // TODO this is really ugly. Feature
> request for 1.5: change auto-component that
> -                               // they don't need to be removed anymore.
> -                               if (!(component instanceof
> InlineEnclosure) && component.isAuto())
> -                               {
> -                                       children_remove(i);
> -                               }
>

auto components should be removed at the end of the rendering.
what has changed so they can stay ?
if add() is used then the auto components are resolved at rendering time
with IComponentResolver, not with IAutoComponentFactory (as with queue()).


>                         }
>                 }
> -
> -               if (children instanceof ChildList)
> -               {
> -                       ChildList lst = (ChildList)children;
> -                       Object[] tmp = new Object[lst.size];
> -                       System.arraycopy(lst.childs, 0, tmp, 0, lst.size);
> -                       children = tmp;
> -               }
>

I don't understand why this is no more needed too...


>         }
>
>         /**
> @@ -1986,7 +1991,35 @@ public abstract class MarkupContainer extends
> Component implements Iterable<Comp
>                         queue = new ComponentQueue();
>                 }
>                 queue.add(components);
> -
> +
> +               return innerQueue();
> +       }
> +
> +       /**
> +        * Queues components over the current container and using the
> given component queue.
> +        *
> +        * @param queue
> +        *                      the component queue to use.
> +        * @return
>

what is the returned value ? this, parent, child ?
I'd like new code to have good javadoc.
JDK 8 javadoc verifier is much stricter than before. It is currently
disabled and it will be quite hard to enable it if we add more "broken"
Javadoc


> +        */
> +       protected MarkupContainer queue(ComponentQueue queue)
> +       {
> +               ComponentQueue currentQueue = this.queue;
> +               this.queue = queue;
> +
> +               MarkupContainer markupContainer = innerQueue();
> +               this.queue = currentQueue;
> +
> +               return markupContainer;
> +       }
> +
> +       /**
> +        * Runs the actual queuing process.
> +        *
> +        * @return
>

what is the return value ?


> +        */
> +       protected MarkupContainer innerQueue()
> +       {
>                 MarkupContainer region = null;
>                 Page page = null;
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/8953bd2c/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java
> b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java
> index 79edb52..5e66d95 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/RelativePathPrefixHandler.java
> @@ -17,6 +17,7 @@
>  package org.apache.wicket.markup.parser.filter;
>
>  import java.text.ParseException;
> +import java.util.concurrent.atomic.AtomicInteger;
>
>  import org.apache.wicket.Component;
>  import org.apache.wicket.MarkupContainer;
> @@ -99,10 +100,9 @@ public final class RelativePathPrefixHandler extends
> AbstractMarkupFilter
>         /**
>          * https://issues.apache.org/jira/browse/WICKET-5724
>          *
> -        * Says if we are inside an head tag or wicket:head tag.
> -        *
> +        * Unique index to generate new tag ids.
>          * */
> -       private boolean insideHead;
> +       private final AtomicInteger componentIndex = new AtomicInteger();
>
>         /**
>          * Constructor for the IComponentResolver role.
> @@ -128,21 +128,9 @@ public final class RelativePathPrefixHandler extends
> AbstractMarkupFilter
>         {
>                 if (tag.isClose())
>                 {
> -                       if (isHeadTag(tag))
> -                       {
> -                               //outside head tag
> -                               insideHead = false;
> -                       }
> -
>                         return tag;
>                 }
>
> -               if (isHeadTag(tag))
> -               {
> -                       //inside head tag
> -                       insideHead = true;
> -               }
> -
>                 String wicketIdAttr = getWicketNamespace() + ":" + "id";
>
>                 // Don't touch any wicket:id component and any
> auto-components
> @@ -162,60 +150,31 @@ public final class RelativePathPrefixHandler extends
> AbstractMarkupFilter
>                         {
>                                 if (tag.getId() == null)
>                                 {
> -
>  tag.setId(getWicketRelativePathPrefix(null));
> +
>  tag.setId(getWicketRelativePathPrefix(null)
> +                                               +
> componentIndex.getAndIncrement());
>                                         tag.setAutoComponentTag(true);
> -
> -                                       /**
> -                                        *
> https://issues.apache.org/jira/browse/WICKET-5724
> -                                        * Transparent component inside
> page body must allow
> -                                        * queued children components.
> -                                        */
> -                                       if(!insideHead)
> -                                       {
> -
>  tag.setAutoComponentFactory(new ComponentTag.IAutoComponentFactory()
> -                                               {
> -                                                       @Override
> -                                                       public Component
> newComponent(MarkupContainer container, ComponentTag tag)
> -                                                       {
> -                                                               String id
> = tag.getId() + container.getPage().getAutoIndex();
> -
>  tag.setId(id);
> -
> -                                                               return new
> TransparentWebMarkupContainer(id);
> -                                                       }
> -                                               });
> -                                       }
>                                 }
> +
>                                 tag.addBehavior(RELATIVE_PATH_BEHAVIOR);
>                                 tag.setModified(true);
> +
>                                 break;
>                         }
>                 }
>
>                 return tag;
>         }
> -
> -       private boolean isHeadTag(ComponentTag tag)
> -       {
> -               if
> (HtmlHeaderSectionHandler.HEAD.equalsIgnoreCase(tag.getName()))
> -               {
> -                       return true;
> -               }
>
> -               return false;
> -       }
> -
>         @Override
>         public Component resolve(final MarkupContainer container, final
> MarkupStream markupStream,
>                 final ComponentTag tag)
>         {
> -               if ((tag != null) &&
> (tag.getId().equals(getWicketRelativePathPrefix(markupStream))))
> +               if ((tag != null) &&
> (tag.getId().startsWith(getWicketRelativePathPrefix(markupStream))))
>                 {
> -                       String id = tag.getId() +
> container.getPage().getAutoIndex();
> -
>                         // we do not want to mess with the hierarchy, so
> the container has to be
>                         // transparent as it may have wicket components
> inside. for example a raw anchor tag
>                         // that contains a label.
> -                       return new TransparentWebMarkupContainer(id);
> +                       return new
> TransparentWebMarkupContainer(tag.getId());
>                 }
>                 return null;
>         }
>
>

Reply via email to