javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-1527

Facelets: Duplicate lclient Ids for components 'stamped' by c:ForEach

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.1_gf31_m4
    • Component/s: facelets
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      1,527
    • Status Whiteboard:
      Hide

      size_medium importance_large

      Show
      size_medium importance_large

      Description

      If you repeat a component with c:forEach in Facelets, the current implementation
      does not ensure that component's client Id is unique when Id is supplied as a
      literal. Consider the following example:

      <c:forEach var="person" items="$

      {people.people}

      ">
      <h:inputText id="item1" value=#

      {person.name}

      />
      </c:forEach>

      This will produce duplicate client Ids with Facelets, but not with JSP.
      If the id attribute is not specified, the unique Ids are generated.
      While I understand that specifying literal Ids on the stamped components appears
      to be a corner case (the client Ids will be impossible to predict anyways), we
      have a real use case where component's Id attribute is used for integration with
      an external content customization solution.

      1. changebundle.txt
        8 kB
        rogerk
      2. foreach.diff
        5 kB
        mst_70

        Issue Links

          Activity

          Hide
          mst_70 added a comment -

          Created an attachment (id=1084)
          Patch for auto-generating Ids after the first stamped component

          Show
          mst_70 added a comment - Created an attachment (id=1084) Patch for auto-generating Ids after the first stamped component
          Hide
          Ryan Lubke added a comment -

          Update target milestone.

          Show
          Ryan Lubke added a comment - Update target milestone.
          Hide
          Ed Burns added a comment -

          ADF keywords

          Show
          Ed Burns added a comment - ADF keywords
          Hide
          Ed Burns added a comment -

          keywords separated by spaces

          Show
          Ed Burns added a comment - keywords separated by spaces
          Hide
          Ed Burns added a comment -

          triage

          Show
          Ed Burns added a comment - triage
          Hide
          Ed Burns added a comment -

          rogerk not roger

          Show
          Ed Burns added a comment - rogerk not roger
          Hide
          rogerk added a comment -

          importance_large

          Show
          rogerk added a comment - importance_large
          Hide
          rogerk added a comment -

          triage

          Show
          rogerk added a comment - triage
          Hide
          rogerk added a comment -

          target 2.1_gf31_m4

          Show
          rogerk added a comment - target 2.1_gf31_m4
          Hide
          Ed Burns added a comment -

          Target for GlassFish 3.1 M4

          Show
          Ed Burns added a comment - Target for GlassFish 3.1 M4
          Hide
          rogerk added a comment -

          starting

          Show
          rogerk added a comment - starting
          Hide
          rogerk added a comment -

          Created an attachment (id=1215)
          Changes

          Show
          rogerk added a comment - Created an attachment (id=1215) Changes
          Hide
          josefreire added a comment -

          Just a heads up.

          This issue was identified on the facelets project, and my latest proposed patch
          is very simple with very good results.

          Please check https://facelets.dev.java.net/issues/show_bug.cgi?id=353, or the
          latest proposed patch
          https://facelets.dev.java.net/nonav/issues/showattachment.cgi/145/proposedPatch-353-20091229.diff

          Thanks.

          Show
          josefreire added a comment - Just a heads up. This issue was identified on the facelets project, and my latest proposed patch is very simple with very good results. Please check https://facelets.dev.java.net/issues/show_bug.cgi?id=353 , or the latest proposed patch https://facelets.dev.java.net/nonav/issues/showattachment.cgi/145/proposedPatch-353-20091229.diff Thanks.
          Hide
          rogerk added a comment -

          Hey thanks Jose,

          I'll take a look at your patch as well.
          Stay tuned....

          -roger

          Show
          rogerk added a comment - Hey thanks Jose, I'll take a look at your patch as well. Stay tuned.... -roger
          Hide
          rogerk added a comment -

          Hello Jose,

          Your patch is really specific to Facelets (pre JSF 2.0 Facelets)..
          Things have changed a bit for JSF 2.0 . Can you rework your patch
          for JSF 2.0 Facelets? This issue is addressing JSF 2.0.

          Show
          rogerk added a comment - Hello Jose, Your patch is really specific to Facelets (pre JSF 2.0 Facelets).. Things have changed a bit for JSF 2.0 . Can you rework your patch for JSF 2.0 Facelets? This issue is addressing JSF 2.0.
          Hide
          josefreire added a comment -

          Hi Roger,

          Sure, I'll give my best shot. I think the approach I took in Facelets 1.1 is
          still valid for JSF 2.0.

          My first patches seem very similar to your patch. However I've found that with
          this approach it broke state saving in some situations (ex: richfaces tree
          stopped working correctly with server state saving).

          Show
          josefreire added a comment - Hi Roger, Sure, I'll give my best shot. I think the approach I took in Facelets 1.1 is still valid for JSF 2.0. My first patches seem very similar to your patch. However I've found that with this approach it broke state saving in some situations (ex: richfaces tree stopped working correctly with server state saving).
          Hide
          Ed Burns added a comment -

          Roger, because you are committing a patch supplied by Stevan, you can
          give the r=rogerk yourself. For what it's worth, I have reviewed it as
          well and give it r=edburns.

          Ed

          Show
          Ed Burns added a comment - Roger, because you are committing a patch supplied by Stevan, you can give the r=rogerk yourself. For what it's worth, I have reviewed it as well and give it r=edburns. Ed
          Hide
          rogerk added a comment -

          checked in.

          Show
          rogerk added a comment - checked in.
          Hide
          josefreire added a comment -

          Hi everyone,

          First a clarification: I confused this bug report with another one that is
          present in Facelets 1.1
          (https://facelets.dev.java.net/issues/show_bug.cgi?id=353), and it's still
          present in JSF 2.0. I'll open a new issue to address it, and I've got a proposed
          patch to upload.

          Anyway, reading carefully this bug description I don't understand why this was
          considered a bug in the first place.

          The example code to illustrate the bug clearly forces the h:inputText to have
          the same ID, resulting in many h:inputText with the same id in the component
          tree, so in my opinion, this is clearly a programming error and the reference
          implementation shouldn't be patched to handle this kind of situations.

          I think that the real bug here is why it doesn't give a duplicate ID with JSP.

          Just my 2 cents...

          Show
          josefreire added a comment - Hi everyone, First a clarification: I confused this bug report with another one that is present in Facelets 1.1 ( https://facelets.dev.java.net/issues/show_bug.cgi?id=353 ), and it's still present in JSF 2.0. I'll open a new issue to address it, and I've got a proposed patch to upload. Anyway, reading carefully this bug description I don't understand why this was considered a bug in the first place. The example code to illustrate the bug clearly forces the h:inputText to have the same ID, resulting in many h:inputText with the same id in the component tree, so in my opinion, this is clearly a programming error and the reference implementation shouldn't be patched to handle this kind of situations. I think that the real bug here is why it doesn't give a duplicate ID with JSP. Just my 2 cents...
          Hide
          kurtz_mr added a comment -

          Is this fix really going in the right direction? I have

          <c:forEach items="#

          {widgetController.widgets}

          " var="widget">
          <ui:include src="#

          {widget.path}

          ">
          <ui:param name="widget" value="#

          {widget}

          "/>
          </ui:include>
          </c:forEach>

          Where I ensure myself ids in included facelets are unique. Too bad framework outsmarts me and replaces my ids with random garbage so I'm unable to do partial updates now.

          Show
          kurtz_mr added a comment - Is this fix really going in the right direction? I have <c:forEach items="# {widgetController.widgets} " var="widget"> <ui:include src="# {widget.path} "> <ui:param name="widget" value="# {widget} "/> </ui:include> </c:forEach> Where I ensure myself ids in included facelets are unique. Too bad framework outsmarts me and replaces my ids with random garbage so I'm unable to do partial updates now.
          Hide
          Manfred Riem added a comment -

          Closing resolved issue out

          Show
          Manfred Riem added a comment - Closing resolved issue out
          Hide
          josefreire added a comment -

          This "fix" is a major regression and is a showstopper to our upgrade from JSF 1.2.

          Issue http://java.net/jira/browse/JAVASERVERFACES-2188 is a clear example on how the effort to make this improvement broke the framework.

          This issue should be reopened and a different approach should be considered.

          <ui:include> should have some kind of flag to signal the framework to rewrite it's id's, and this should not be the default behaviour.

          Show
          josefreire added a comment - This "fix" is a major regression and is a showstopper to our upgrade from JSF 1.2. Issue http://java.net/jira/browse/JAVASERVERFACES-2188 is a clear example on how the effort to make this improvement broke the framework. This issue should be reopened and a different approach should be considered. <ui:include> should have some kind of flag to signal the framework to rewrite it's id's, and this should not be the default behaviour.
          Hide
          josefreire added a comment -

          There's a trivial workaround for this issue.

          If the user wraps the <ui:include> with <f:subView>, there will be no id clash:

          <c:forEach items="#

          {controller.list}

          " var="itemToInclude">
          <f:subView>
          <ui:include src="#

          {itemToInclude}

          " />
          </f:subView>
          </c:forEach>

          The user clearly doesn't require the ids, so a f:subView is a reasonable and easy solution.

          Could you please reopen this issue?

          Show
          josefreire added a comment - There's a trivial workaround for this issue. If the user wraps the <ui:include> with <f:subView>, there will be no id clash: <c:forEach items="# {controller.list} " var="itemToInclude"> <f:subView> <ui:include src="# {itemToInclude} " /> </f:subView> </c:forEach> The user clearly doesn't require the ids, so a f:subView is a reasonable and easy solution. Could you please reopen this issue?

            People

            • Assignee:
              rogerk
              Reporter:
              mst_70
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: