Repository: wicket Updated Branches: refs/heads/wicket-6.x e3f1b4e39 -> 381391bc2
WICKET-6219 testing the tag name to prevent conflict when resolving fragments and components markup Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/381391bc Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/381391bc Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/381391bc Branch: refs/heads/wicket-6.x Commit: 381391bc2d4ae7567a385987152687453c213121 Parents: e3f1b4e Author: Pedro Henrique Oliveira dos Santos <pe...@apache.org> Authored: Fri Aug 19 03:09:59 2016 -0300 Committer: Pedro Henrique Oliveira dos Santos <pe...@apache.org> Committed: Fri Aug 19 03:09:59 2016 -0300 ---------------------------------------------------------------------- .../wicket/markup/AbstractMarkupFragment.java | 87 +------------------ .../apache/wicket/markup/IMarkupFragment.java | 10 +-- .../java/org/apache/wicket/markup/TagUtils.java | 88 ++++++++++++++++++++ .../html/TransparentWebMarkupContainer.java | 8 +- .../panel/FragmentMarkupSourcingStrategy.java | 3 +- .../html/TransparentWebMarkupContainerTest.java | 68 ++++++++++++++- 6 files changed, 170 insertions(+), 94 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/381391bc/wicket-core/src/main/java/org/apache/wicket/markup/AbstractMarkupFragment.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/AbstractMarkupFragment.java b/wicket-core/src/main/java/org/apache/wicket/markup/AbstractMarkupFragment.java index b9de2a9..e1bc94e 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/AbstractMarkupFragment.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/AbstractMarkupFragment.java @@ -16,98 +16,15 @@ */ package org.apache.wicket.markup; -import java.util.Deque; -import java.util.LinkedList; - -import org.apache.wicket.util.lang.Args; - /** * A base implementation of {@link IMarkupFragment}. */ public abstract class AbstractMarkupFragment implements IMarkupFragment { - /** - * Find the markup fragment of component with id equal to {@code id}, starting at offset {@code streamOffset}. - * - * @param id - * The id of the component tag being searched for. - * @param streamOffset - * The offset in the markup stream from which to start searching. - * @return the {@link IMarkupFragment} of the component tag if found, {@code null} is not found. - */ + protected final IMarkupFragment find(final String id, int streamOffset) { - /* - * We need streamOffset because MarkupFragment starts searching from offset 1. - */ - Args.notEmpty(id, "id"); - Args.withinRange(0, size() - 1, streamOffset, "streamOffset"); - - Deque<Boolean> openTagUsability = new LinkedList<Boolean>(); - boolean canFind = true; - - MarkupStream stream = new MarkupStream(this); - stream.setCurrentIndex(streamOffset); - while (stream.hasMore()) - { - MarkupElement elem = stream.get(); - - if (elem instanceof ComponentTag) - { - ComponentTag tag = stream.getTag(); - - if (tag.isOpen() || tag.isOpenClose()) - { - if (canFind && tag.getId().equals(id)) - { - return stream.getMarkupFragment(); - } - else if (tag.isOpen() && !tag.hasNoCloseTag()) - { - openTagUsability.push(canFind); - - if (tag instanceof WicketTag) - { - WicketTag wtag = (WicketTag)tag; - - if (wtag.isExtendTag()) - { - canFind = true; - } - else if (wtag.isFragementTag() || wtag.isContainerTag()) - { - canFind = false; - } - /* - * We should potentially also not try find child markup inside some other - * Wicket tags. Other tags that we should think about refusing to look for - * child markup inside include: container, body, border, child (we already - * have special extend handling). - */ - } - else if (!"head".equals(tag.getName()) && !tag.isAutoComponentTag()) - { - canFind = false; - } - } - } - else if (tag.isClose()) - { - if (openTagUsability.isEmpty()) - { - canFind = false; - } - else - { - canFind = openTagUsability.pop(); - } - } - } - - stream.next(); - } - - return null; + return TagUtils.findTagMarkup(this, id, null, streamOffset); } @Override http://git-wip-us.apache.org/repos/asf/wicket/blob/381391bc/wicket-core/src/main/java/org/apache/wicket/markup/IMarkupFragment.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/IMarkupFragment.java b/wicket-core/src/main/java/org/apache/wicket/markup/IMarkupFragment.java index dc070c4..ef51ce9 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/IMarkupFragment.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/IMarkupFragment.java @@ -55,13 +55,13 @@ public interface IMarkupFragment extends Iterable<MarkupElement> int size(); /** - * Find the markup fragment of the component with 'path' + * Finds a markup fragment that spans a tag * * @param id - * The component's id to search for - * @return -1, if not found + * the wicket:id attribute in the tag + * @return the markup fragment that spans the complete found tag */ - IMarkupFragment find(final String id); + IMarkupFragment find(final String wicketId); /** * @@ -70,4 +70,4 @@ public interface IMarkupFragment extends Iterable<MarkupElement> * @return markup string */ String toString(final boolean markupOnly); -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/wicket/blob/381391bc/wicket-core/src/main/java/org/apache/wicket/markup/TagUtils.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/TagUtils.java b/wicket-core/src/main/java/org/apache/wicket/markup/TagUtils.java index d1168f2..6cac731 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/TagUtils.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/TagUtils.java @@ -16,7 +16,11 @@ */ package org.apache.wicket.markup; +import java.util.Deque; +import java.util.LinkedList; + import org.apache.wicket.MarkupContainer; +import org.apache.wicket.util.lang.Args; import org.apache.wicket.util.value.IValueMap; import org.apache.wicket.util.value.ValueMap; @@ -213,4 +217,88 @@ public class TagUtils "Expected a Tag but found raw markup: " + elem.toString()); } } + + /** + * Find the markup fragment of a tag with wicket:id equal to {@code id} starting at offset {@code streamOffset}. + * + * @param id + * The wicket:id of the tag being searched for. + * @param tagName + * The tag name of the tag being searched for. + * @param streamOffset + * The offset in the markup stream from which to start searching. + * @return the {@link IMarkupFragment} of the component tag if found, {@code null} is not found. + */ + public static final IMarkupFragment findTagMarkup(IMarkupFragment fragment, String id, String tagName, int streamOffset) + { + /* + * We need streamOffset because MarkupFragment starts searching from offset 1. + */ + Args.notEmpty(id, "id"); + Args.withinRange(0, fragment.size() - 1, streamOffset, "streamOffset"); + + Deque<Boolean> openTagUsability = new LinkedList<Boolean>(); + boolean canFind = true; + + MarkupStream stream = new MarkupStream(fragment); + stream.setCurrentIndex(streamOffset); + while (stream.hasMore()) + { + MarkupElement elem = stream.get(); + + if (elem instanceof ComponentTag) + { + ComponentTag tag = stream.getTag(); + + if (tag.isOpen() || tag.isOpenClose()) + { + if (canFind && tag.getId().equals(id) && (tagName==null || tag.getName().equals(tagName))) + { + return stream.getMarkupFragment(); + } + else if (tag.isOpen() && !tag.hasNoCloseTag()) + { + openTagUsability.push(canFind); + + if (tag instanceof WicketTag) + { + WicketTag wtag = (WicketTag)tag; + + if (wtag.isExtendTag()) + { + canFind = true; + } + else if (wtag.isFragementTag() || wtag.isContainerTag()) + { + canFind = false; + } + /* + * We should potentially also not try find child markup inside some other + * Wicket tags. Other tags that we should think about refusing to look for + * child markup inside include: container, body, border, child (we already + * have special extend handling). + */ + } + else if (!"head".equals(tag.getName()) && !tag.isAutoComponentTag()) + { + canFind = false; + } + } + } + else if (tag.isClose()) + { + if (openTagUsability.isEmpty()) + { + canFind = false; + } + else + { + canFind = openTagUsability.pop(); + } + } + } + stream.next(); + } + return null; + } } http://git-wip-us.apache.org/repos/asf/wicket/blob/381391bc/wicket-core/src/main/java/org/apache/wicket/markup/html/TransparentWebMarkupContainer.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/TransparentWebMarkupContainer.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/TransparentWebMarkupContainer.java index 9125528..547fcda 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/TransparentWebMarkupContainer.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/TransparentWebMarkupContainer.java @@ -54,6 +54,12 @@ public class TransparentWebMarkupContainer extends WebMarkupContainer implements @Override public Component resolve(MarkupContainer container, MarkupStream markupStream, ComponentTag tag) { + if ("wicket".equals(tag.getNamespace()) && "fragment".equals(tag.getName())) + { + // even having a wicket:id it isn't a component's markup + return null; + } + Component resolvedComponent = getParent().get(tag.getId()); if (resolvedComponent != null && getPage().wasRendered(resolvedComponent)) { @@ -154,4 +160,4 @@ public class TransparentWebMarkupContainer extends WebMarkupContainer implements } } } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/wicket/blob/381391bc/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/FragmentMarkupSourcingStrategy.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/FragmentMarkupSourcingStrategy.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/FragmentMarkupSourcingStrategy.java index 8d4c53b..015cd1f 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/FragmentMarkupSourcingStrategy.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/panel/FragmentMarkupSourcingStrategy.java @@ -25,6 +25,7 @@ import org.apache.wicket.markup.MarkupElement; import org.apache.wicket.markup.MarkupException; import org.apache.wicket.markup.MarkupNotFoundException; import org.apache.wicket.markup.MarkupStream; +import org.apache.wicket.markup.TagUtils; import org.apache.wicket.markup.WicketTag; import org.apache.wicket.util.lang.Args; @@ -124,7 +125,7 @@ public class FragmentMarkupSourcingStrategy extends AbstractMarkupSourcingStrate } // Search for the fragment markup - IMarkupFragment childMarkup = markup.find(markupId); + IMarkupFragment childMarkup = TagUtils.findTagMarkup(markup, markupId, "fragment", 1 ); if (childMarkup == null) { // There is one more option if the markup provider has associated markup http://git-wip-us.apache.org/repos/asf/wicket/blob/381391bc/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java index 8f2cafa..e8e7984 100644 --- a/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/TransparentWebMarkupContainerTest.java @@ -16,21 +16,25 @@ */ package org.apache.wicket.markup.html; +import static org.hamcrest.Matchers.containsString; + import org.apache.wicket.Component; import org.apache.wicket.IPageManagerProvider; import org.apache.wicket.MarkupContainer; import org.apache.wicket.WicketTestCase; import org.apache.wicket.ajax.AjaxRequestTarget; import org.apache.wicket.ajax.markup.html.AjaxLink; +import org.apache.wicket.core.util.lang.WicketObjects; import org.apache.wicket.markup.IMarkupResourceStreamProvider; import org.apache.wicket.markup.MarkupException; import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.border.Border; +import org.apache.wicket.markup.html.panel.Fragment; import org.apache.wicket.mock.MockPageManager; import org.apache.wicket.page.IManageablePage; import org.apache.wicket.page.IPageManager; import org.apache.wicket.page.IPageManagerContext; -import org.apache.wicket.core.util.lang.WicketObjects; +import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.util.resource.IResourceStream; import org.apache.wicket.util.resource.StringResourceStream; import org.apache.wicket.util.tester.TagTester; @@ -151,7 +155,17 @@ public class TransparentWebMarkupContainerTest extends WicketTestCase //check if our response contains headers assertNotNull(scriptTag); } - + + /** + * https://issues.apache.org/jira/browse/WICKET-6219 + */ + @Test + public void shouldAllowAFragmentIdConflictingToASibilingTagWicketId() throws Exception + { + tester.startPage(SubPageWithAFragment.class); + assertThat(tester.getLastResponseAsString(), containsString("content")); + } + /** */ public static class TestPage extends WebPage implements IMarkupResourceStreamProvider { @@ -282,4 +296,54 @@ public class TransparentWebMarkupContainerTest extends WicketTestCase "</body></html>"); } } + public static class PageWithAChildInsideATransparentContainer extends WebPage + implements + IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + public PageWithAChildInsideATransparentContainer(PageParameters parameters) + { + super(parameters); + add(new TransparentWebMarkupContainer("wrapper")); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("" + // + "<html><body>" + // + " <div wicket:id=\"wrapper\">" + // + " <wicket:child/>" + // + " </div>" + // + "</body></html>"); + } + } + public static class SubPageWithAFragment extends PageWithAChildInsideATransparentContainer + { + private static final long serialVersionUID = 1L; + + public SubPageWithAFragment(PageParameters parameters) + { + super(parameters); + Fragment fragment = new Fragment("header", "header", this); + add(fragment); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + if (PageWithAChildInsideATransparentContainer.class.equals(containerClass)) + return super.getMarkupResourceStream(container, containerClass); + return new StringResourceStream("" + // + "<html><body>" + // + "<wicket:extend>" + // + " <div wicket:id=\"header\"></div>" + // + " <wicket:fragment wicket:id=\"header\">content</wicket:fragment>" + // + "</wicket:extend>" + // + "</body></html>"); + } + } }
