Repository: wicket Updated Branches: refs/heads/improvingQueuing [created] 6b879633f
WICKET-5730 Dequeue auto component can't resolve components if they are nested in child markup Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/6b879633 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/6b879633 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/6b879633 Branch: refs/heads/improvingQueuing Commit: 6b879633f7d6f4cde571f510d8cc7ed4f09da1bb Parents: 1d64d4c Author: Andrea Del Bene <[email protected]> Authored: Tue Oct 21 12:47:38 2014 +0200 Committer: Andrea Del Bene <[email protected]> Committed: Tue Oct 21 18:13:14 2014 +0200 ---------------------------------------------------------------------- .../main/java/org/apache/wicket/Component.java | 6 +- .../java/org/apache/wicket/DequeueContext.java | 6 +- .../java/org/apache/wicket/MarkupContainer.java | 233 +++++++++---------- .../org/apache/wicket/markup/ComponentTag.java | 23 +- .../wicket/markup/html/border/Border.java | 7 - .../markup/parser/filter/EnclosureHandler.java | 6 +- .../markup/parser/filter/HtmlHandler.java | 33 ++- .../filter/RelativePathPrefixHandler.java | 48 +++- 8 files changed, 209 insertions(+), 153 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/wicket-core/src/main/java/org/apache/wicket/Component.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java b/wicket-core/src/main/java/org/apache/wicket/Component.java index 29cafa5..fb69ff0 100644 --- a/wicket-core/src/main/java/org/apache/wicket/Component.java +++ b/wicket-core/src/main/java/org/apache/wicket/Component.java @@ -397,9 +397,9 @@ public abstract class Component * Flag that makes we are in before-render callback phase Set after component.onBeforeRender is * invoked (right before invoking beforeRender on children) */ - private static final int FLAG_RENDERING = 0x2000000; - private static final int FLAG_PREPARED_FOR_RENDER = 0x4000000; - private static final int FLAG_AFTER_RENDERING = 0x8000000; + protected static final int FLAG_RENDERING = 0x2000000; + protected static final int FLAG_PREPARED_FOR_RENDER = 0x4000000; + protected static final int FLAG_AFTER_RENDERING = 0x8000000; /** * Flag that restricts visibility of a component when set to true. This is usually used when a http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java b/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java index 1b35152..3168739 100644 --- a/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java +++ b/wicket-core/src/main/java/org/apache/wicket/DequeueContext.java @@ -67,8 +67,8 @@ public final class DequeueContext { this.markup = markup; this.skipFirst = skipFirst; - containers.push(root); - next=nextTag(); + this.containers.push(root); + this.next=nextTag(); } /** @@ -279,7 +279,7 @@ public final class DequeueContext } /** - * Searches the container stack for a component that can be dequeud + * Searches the container stack for a component that can be dequeude * * @param tag * @return http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/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 f8f9116..8a15571 100644 --- a/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java +++ b/wicket-core/src/main/java/org/apache/wicket/MarkupContainer.java @@ -262,10 +262,6 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp } add(component); - /** - * https://issues.apache.org/jira/browse/WICKET-5724 - */ - return true; } @@ -932,34 +928,22 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp ComponentStrings.toString(child, new MarkupException("added"))); } - Page page = null; - MarkupContainer queueRegion = null; - Component cursor = this; - while (cursor != null) - { - if (queueRegion == null && (cursor instanceof IQueueRegion)) - { - queueRegion = (MarkupContainer)cursor; - } - if (cursor instanceof Page) - { - page = (Page)cursor; - } - cursor = cursor.getParent(); - } - - // if we have a path to page dequeue any children - if (page != null) + Page page = findPage(); + + // if we have a path to page dequeue any container children. + // we can do it only if page is not already rendering! + if (page != null && !page.getFlag(FLAG_RENDERING) && child instanceof MarkupContainer) { - // if we are already dequeueing there is no need to dequeue again - if (!queueRegion.getRequestFlag(RFLAG_CONTAINER_DEQUEING)) + MarkupContainer childContainer = (MarkupContainer)child; + // if we are already dequeueing there is no need to dequeue again + if (!childContainer.getRequestFlag(RFLAG_CONTAINER_DEQUEING)) { /* * dequeue both normal and auto components * * https://issues.apache.org/jira/browse/WICKET-5724 */ - queueRegion.dequeue(); + childContainer.dequeue(); } } @@ -1520,11 +1504,7 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp protected void onInitialize() { super.onInitialize(); - if (this instanceof IQueueRegion) - { - // if this container is a queue region dequeue any queued up auto components - dequeueAutoComponents(); - } + dequeueAutoComponents(); } private void dequeueAutoComponents() @@ -1535,11 +1515,18 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp { for (ComponentTag tag = context.takeTag(); tag != null; tag = context.takeTag()) { - ComponentTag.IAutoComponentFactory autoComponentFactory = tag.getAutoComponentFactory(); - if (autoComponentFactory != null) - { - queue(autoComponentFactory.newComponent(this, tag)); - } + ComponentTag.IAutoComponentFactory autoComponentFactory = tag.getAutoComponentFactory(); + if (autoComponentFactory != null) + { + queue(autoComponentFactory.newComponent(this, tag)); + } + + // Every component is responsible just for its own auto components + // so skip to the close tag. + if (tag.isOpen() && !tag.hasNoCloseTag()) + { + context.skipToCloseTag(); + } } } } @@ -1964,6 +1951,7 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp * freedom when moving components in markup. * * @param components + * the components to queue * @return {@code this} for method chaining */ public MarkupContainer queue(Component... components) @@ -1974,40 +1962,11 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp } queue.add(components); - return internalQueue(); - } - - /** - * Runs the actual queuing process. - * - * @return {@code this} for method chaining - */ - private MarkupContainer internalQueue() - { - MarkupContainer region = null; - Page page = null; - - MarkupContainer cursor = this; - - while (cursor != null) - { - if (region == null && cursor instanceof IQueueRegion) - { - region = cursor; - } - if (cursor instanceof Page) - { - page = (Page)cursor; - } - cursor = cursor.getParent(); - } + Page page = findPage(); if (page != null) { - if (!region.getRequestFlag(RFLAG_CONTAINER_DEQUEING)) - { - region.dequeue(); - } + dequeue(); } return this; @@ -2018,12 +1977,33 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp */ public void dequeue() { - if (!(this instanceof IQueueRegion)) - { - throw new UnsupportedOperationException( - "Only implementations of IQueueRegion can use component queueing"); - } - + if (this instanceof IQueueRegion) + { + DequeueContext dequeue = newDequeueContext(); + dequeuePreamble(dequeue); + } + else + { + MarkupContainer queueRegion = (MarkupContainer)findParent(IQueueRegion.class); + + if (!queueRegion.getRequestFlag(RFLAG_CONTAINER_DEQUEING)) + { + queueRegion.dequeue(); + } + } + } + + /** + * Run preliminary operations before running {@link #dequeue(DequeueContext)}. More in detail + * it throws an exception if the container is already dequeuing, and it also takes care of + * setting flag {@code RFLAG_CONTAINER_DEQUEING} to true before running {@link #dequeue(DequeueContext)} + * and setting it back to false after dequeuing is completed. + * + * @param dequeue + * the dequeue context to use + */ + protected void dequeuePreamble(DequeueContext dequeue) + { if (getRequestFlag(RFLAG_CONTAINER_DEQUEING)) { throw new IllegalStateException("This container is already dequeing: " + this); @@ -2032,10 +2012,8 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp setRequestFlag(RFLAG_CONTAINER_DEQUEING, true); try { - DequeueContext dequeue = newDequeueContext(); if (dequeue == null) { - // not ready to dequeue yet return; } @@ -2056,14 +2034,12 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp * components in queues filled by a call to {@link #queue(Component...)}. It then delegates the * dequeueing to these children. * - * The provided {@link DequeueContext} is used to maintain the place in markup as well as the - * stack of components whose queues will be searched. For example, before delegating the call to - * a child the container will push the child onto the stack of components. * * Certain components that implement custom markup behaviors (such as repeaters and borders) * override this method to bring dequeueing in line with their custom markup handling. * * @param dequeue + * the dequeue context to use */ public void dequeue(DequeueContext dequeue) { @@ -2072,77 +2048,76 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp ComponentTag tag = dequeue.takeTag(); // see if child is already added to parent - Component child = get(tag.getId()); if (child == null) { // the container does not yet have a child with this id, see if we can // dequeue - child = dequeue.findComponentToDequeue(tag); if (child != null) { - addDequeuedComponent(child, tag); - if (child instanceof IQueueRegion) - { - ((MarkupContainer)child).dequeue(); - } - } - } - if (child == null || !(child instanceof MarkupContainer)) - { - // could not dequeue, or does not contain children - - if (tag.isOpen() && !tag.hasNoCloseTag()) - { - dequeue.skipToCloseTag(); - } - } - else - { - MarkupContainer container = (MarkupContainer)child; - if (container instanceof IQueueRegion) - { - // if this is a dequeue container we do not process its markup, it will do so - // itself when it is dequeued for the first time - if (tag.isOpen()) - { - dequeue.skipToCloseTag(); - } - } - else if (tag.isOpen()) - { - // this component has more markup and possibly more children to dequeue - dequeue.pushContainer(container); - container.dequeue(dequeue); - dequeue.popContainer(); + addDequeuedComponent(child, tag); } } - + if (tag.isOpen() && !tag.hasNoCloseTag()) - { - // pull the close tag off - ComponentTag close = dequeue.takeTag(); - if (!close.closes(tag)) - { - // sanity check - throw new IllegalStateException(String.format("Tag '%s' should be the closing one for '%s'", close, tag)); - } - } + { + dequeueChild(child, tag, dequeue); + } } } - /** @see IQueueRegion#newDequeueContext() */ + /** + * Propagates dequeuing to children components. + * + * @param child + * the children component + * @param tag + * the children tag + * @param dequeue + * the dequeue context to use + */ + private void dequeueChild(Component child, ComponentTag tag, DequeueContext dequeue) + { + if (child == null || child instanceof IQueueRegion) + { + // could not dequeue, or is a dequeue container + dequeue.skipToCloseTag(); + + } + else if (child instanceof MarkupContainer) + { + //propagate dequeuing to containers + MarkupContainer childContainer = (MarkupContainer)child; + + dequeue.pushContainer(childContainer); + childContainer.dequeue(dequeue); + dequeue.popContainer(); + } + + // pull the close tag off + ComponentTag close = dequeue.takeTag(); + if (!close.closes(tag)) + { + // sanity check + throw new IllegalStateException(String.format("Tag '%s' should be the closing one for '%s'", close, tag)); + } + + } + + /** @see IQueueRegion#newDequeueContext() */ public DequeueContext newDequeueContext() { - Markup markup = getAssociatedMarkup(); + IMarkupFragment markup = getAssociatedMarkup(); + if (markup == null) - { - return null; - } + { + return null; + } + return new DequeueContext(markup, this, false); } @@ -2159,7 +2134,7 @@ public abstract class MarkupContainer extends Component implements Iterable<Comp */ protected DequeueTagAction canDequeueTag(ComponentTag tag) { - if (tag instanceof WicketTag) + if (tag instanceof WicketTag) { WicketTag wicketTag = (WicketTag)tag; if (wicketTag.isContainerTag()) http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java b/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java index d93d3e5..08caf0d 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/ComponentTag.java @@ -92,6 +92,12 @@ public class ComponentTag extends MarkupElement /** Render the tag as RawMarkup even if no Component can be found */ public final static int RENDER_RAW = 0x0020; + + /** + * If true, the component is the parent (or the ancestor) of Wicket component explicitly added. + * (i.e. it contains child tag with a "wicket:id" attribute) + * */ + public final static int CONTAINS_WICKET_ID = 0x0040; /** If close tag, than reference to the corresponding open tag */ private ComponentTag openTag; @@ -827,7 +833,22 @@ public class ComponentTag extends MarkupElement { setFlag(NO_CLOSE_TAG, hasNoCloseTag); } - + + /** + * True if the HTML tag (e.g. br) has no close tag + * + * @param hasNoCloseTag + */ + public void setContainsWicketId(boolean containsWicketId) + { + setFlag(CONTAINS_WICKET_ID, containsWicketId); + } + + public boolean containsWicketId() + { + return getFlag(CONTAINS_WICKET_ID); + } + /** * In case of inherited markup, the base and the extended markups are merged and the information * about the tags origin is lost. In some cases like wicket:head and wicket:link this http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/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 1c4a1ec..934ab39 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 @@ -216,13 +216,6 @@ public abstract class Border extends WebMarkupContainer implements IComponentRes } @Override - public Border queue(Component... components) - { - getBodyContainer().queue(components); - return this; - } - - @Override public Border remove(final Component component) { if (component == body) http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/EnclosureHandler.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/EnclosureHandler.java b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/EnclosureHandler.java index 098533c..7739cd0 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/EnclosureHandler.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/EnclosureHandler.java @@ -33,6 +33,7 @@ import org.apache.wicket.markup.WicketTag; import org.apache.wicket.markup.html.internal.Enclosure; import org.apache.wicket.markup.parser.AbstractMarkupFilter; import org.apache.wicket.markup.resolver.IComponentResolver; +import org.apache.wicket.util.string.Strings; /** @@ -155,7 +156,7 @@ public final class EnclosureHandler extends AbstractMarkupFilter implements ICom ComponentTag lastEnclosure = stack.getLast(); // If the enclosure tag has NO child attribute, then ... - if (lastEnclosure.getAttribute(CHILD_ATTRIBUTE) == null) + if (Strings.isEmpty(lastEnclosure.getAttribute(CHILD_ATTRIBUTE))) { String id = tag.getAttribute(getWicketNamespace() + ":id"); if (id != null) @@ -186,8 +187,7 @@ public final class EnclosureHandler extends AbstractMarkupFilter implements ICom if ((tag instanceof WicketTag) && ((WicketTag)tag).isEnclosureTag()) { // Yes, we handled the tag - return new Enclosure(tag.getId() + container.getPage().getAutoIndex(), - tag.getAttribute(EnclosureHandler.CHILD_ATTRIBUTE)); + return new Enclosure(tag.getId(), tag.getAttribute(EnclosureHandler.CHILD_ATTRIBUTE)); } // We were not able to handle the tag http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java index 0d007c1..5801b4e 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/HtmlHandler.java @@ -106,8 +106,11 @@ public final class HtmlHandler extends AbstractMarkupFilter // Check tag type if (tag.isOpen()) { + // Check if open tags contains a "wicket:id" component + setContainsWicketIdFlag(tag); + // Push onto stack - stack.push(tag); + stack.push(tag); } else if (tag.isClose()) { @@ -123,13 +126,12 @@ public final class HtmlHandler extends AbstractMarkupFilter if (mismatch) { - top.setHasNoCloseTag(true); - // Pop any simple tags off the top of the stack while (mismatch && !requiresCloseTag(top.getName())) { top.setHasNoCloseTag(true); - + top.setContainsWicketId(false); + // Pop simple tag if (stack.isEmpty()) { @@ -167,8 +169,29 @@ public final class HtmlHandler extends AbstractMarkupFilter return tag; } - + /** + * Checks if the tag is a Wicket component explicitly added. i.e + * it has the "wicket:id" attribute. + * + * @param tag + */ + private void setContainsWicketIdFlag(ComponentTag tag) + { + //check if it is a wicket:id component + String wicketIdAttr = getWicketNamespace() + ":" + "id"; + boolean hasWicketId = tag.getAttributes().get(wicketIdAttr) != null; + + if (hasWicketId) + { + for (ComponentTag componentTag : stack) + { + componentTag.setContainsWicketId(hasWicketId); + } + } + } + + /** * Gets whether this tag does not require a closing tag. * * @param name http://git-wip-us.apache.org/repos/asf/wicket/blob/6b879633/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 5e66d95..1ab60c8 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,12 +17,15 @@ package org.apache.wicket.markup.parser.filter; import java.text.ParseException; +import java.util.Iterator; import java.util.concurrent.atomic.AtomicInteger; import org.apache.wicket.Component; import org.apache.wicket.MarkupContainer; import org.apache.wicket.behavior.Behavior; import org.apache.wicket.markup.ComponentTag; +import org.apache.wicket.markup.ComponentTag.IAutoComponentFactory; +import org.apache.wicket.markup.Markup; import org.apache.wicket.markup.MarkupElement; import org.apache.wicket.markup.MarkupResourceStream; import org.apache.wicket.markup.MarkupStream; @@ -97,6 +100,16 @@ public final class RelativePathPrefixHandler extends AbstractMarkupFilter } }; + private static final IAutoComponentFactory FACTORY = new IAutoComponentFactory() + { + @Override + public Component newComponent(MarkupContainer container, ComponentTag tag) + { + return new TransparentWebMarkupContainer(tag.getId()); + } + }; + + /** * https://issues.apache.org/jira/browse/WICKET-5724 * @@ -152,7 +165,7 @@ public final class RelativePathPrefixHandler extends AbstractMarkupFilter { tag.setId(getWicketRelativePathPrefix(null) + componentIndex.getAndIncrement()); - tag.setAutoComponentTag(true); + tag.setAutoComponentTag(true); } tag.addBehavior(RELATIVE_PATH_BEHAVIOR); @@ -178,7 +191,38 @@ public final class RelativePathPrefixHandler extends AbstractMarkupFilter } return null; } - + + @Override + public void postProcess(Markup markup) + { + /** + * https://issues.apache.org/jira/browse/WICKET-5724 + * + * Transparent component inside page body must allow + * queued children components. + */ + Iterator<MarkupElement> markupIterator = markup.iterator(); + while (markupIterator.hasNext()) + { + MarkupElement next = markupIterator.next(); + + if(next instanceof ComponentTag) + { + ComponentTag componentTag = (ComponentTag)next; + + /** + * if component tag is for a transparent component + * and contains "wicket:id", must be queueable. + */ + if (componentTag.containsWicketId() && + componentTag.getId().startsWith(getWicketRelativePathPrefix(null))) + { + componentTag.setAutoComponentFactory(FACTORY); + } + } + } + } + private String getWicketRelativePathPrefix(final MarkupStream markupStream) { return getWicketNamespace(markupStream) + WICKET_RELATIVE_PATH_PREFIX_CONTAINER_ID;
