Hi Sven > I double checked with how other filters/resolvers are doing it and I found > AutoLabelTextResolver to leave the tag id untouched too.
which is a fault as well. Same risk. > >>May be a Component's flag bit can be used for that purpose. > > Shouldn't that be a flag on the markup tag instead then? No, because the Pages are made of potentially many markups. Since the ID has no info about hierarchy, the number appended is the only thing that makes it page unique. Hence it can not be per markup. Since every tag gets associated with a Component, the Component is the right place for the flag. >>I don't know how likely it is and I know we have no tests for it. > > Yeah, I didn't find one either. I'm not sure I understand where's a possible > problem here. It's hard to discuss this issues without an actual case. An artifical markup which creates duplicate IDs can be constructed. I've not tried it but I don't expect it to be too difficult. Knowing there is a potential risk which will be very hard to find by a normal user, makes me belief we should fix it properly, To me the applied patch leaves us with a bigger problem than it fixes. Juergen > Sven > > > On 03/29/2012 08:42 PM, Juergen Donnerstag wrote: >> >> Hi Sven, >> >> the change introduces a theoretical risk of duplicate ids. I don't >> know how likely it is and I know we have no tests for it. But Filters >> are created per markup file and modify the markup upon loading, prior >> to caching. Which also means that the Page is not available. In case >> of inheritance it might not even be the same Page. To be absolutely >> sure the ID is page-unique, it gets update in the Resolvers where the >> Page instance is available. >> >> The root cause is that we don't validate if the ID has been updated >> already and avoid subsequent updates. We have no means to do that >> check. A number at the end of the ID would not be sufficient. May be a >> Component's flag bit can be used for that purpose. >> >> I understand the problem in your application, but users will have an >> extremly hard time to identify the issue if it occurs. I'm not sure >> it's worth it. May be the approach outlined above solved your problem >> as well, but in a safe manner. >> >> Juergen >> >> On Thu, Mar 29, 2012 at 6:43 PM,<svenme...@apache.org> wrote: >>> >>> Updated Branches: >>> refs/heads/wicket-1.5.x c6c45a510 -> d7e0d8845 >>> >>> >>> WICKET-4484 do not change tag id when resolving component >>> >>> >>> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo >>> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/d7e0d884 >>> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/d7e0d884 >>> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/d7e0d884 >>> >>> Branch: refs/heads/wicket-1.5.x >>> Commit: d7e0d8845dd509110e48ba965f0e11a3aae6a1f5 >>> Parents: c6c45a5 >>> Author: Sven Meier<svenme...@apache.org> >>> Authored: Thu Mar 29 18:43:07 2012 +0200 >>> Committer: Sven Meier<svenme...@apache.org> >>> Committed: Thu Mar 29 18:43:07 2012 +0200 >>> >>> ---------------------------------------------------------------------- >>> .../markup/parser/filter/WicketLinkTagHandler.java | 1 - >>> 1 files changed, 0 insertions(+), 1 deletions(-) >>> ---------------------------------------------------------------------- >>> >>> >>> >>> http://git-wip-us.apache.org/repos/asf/wicket/blob/d7e0d884/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/WicketLinkTagHandler.java >>> ---------------------------------------------------------------------- >>> diff --git >>> a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/WicketLinkTagHandler.java >>> b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/WicketLinkTagHandler.java >>> index 689133e..8779b6f 100644 >>> --- >>> a/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/WicketLinkTagHandler.java >>> +++ >>> b/wicket-core/src/main/java/org/apache/wicket/markup/parser/filter/WicketLinkTagHandler.java >>> @@ -208,7 +208,6 @@ public class WicketLinkTagHandler extends >>> AbstractMarkupFilter implements ICompo >>> if (wtag.isLinkTag()&& (wtag.getNamespace() != >>> null)) >>> { >>> String id = tag.getId() + "-" + >>> container.getPage().getAutoIndex(); >>> - tag.setId(id); >>> >>> return new >>> TransparentWebMarkupContainer(id); >>> } >>> >
