Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicate ID exception when using composite component with c:if #4244

Closed
ren-zhijun-oracle opened this issue Apr 3, 2017 · 25 comments
Closed

Comments

@ren-zhijun-oracle
Copy link
Contributor

I have a duplicate ID exception when using a composite component two times at same page.
Both components have different IDs.
One component is conditionally rendered with <c:if>, the other one isn't
I don't have this exception when doing exactly the same thing but using a plain JSF component instead.

I already asked at stackoverflow but got no answer:
http://stackoverflow.com/questions/42863043/duplicate-id-exception-when-using-cif-with-composite-component

Below the reproducer code:

A session scoped Bean: TestBean

@Named
@SessionScoped
public class TestBean implements Serializable {

    private boolean isVisible = false;

    public void onSetItemVisible(AjaxBehaviorEvent e) {
        this.isVisible = true;
    }

    public void onSetItemInvisible(AjaxBehaviorEvent e) {
        this.isVisible = false;
    }

    public boolean isItemVisible()  {
        return this.isVisible;
    }
}

A simple composite component: testCmp

<html xmlns="http://www.w3.org/1999/xhtml"
    xmlns:h="http://java.sun.com/jsf/html" 
    xmlns:cc="http://java.sun.com/jsf/composite"> 
    <cc:interface/>
    <cc:implementation>
        <h:outputText id="text" value="text"/>
     </cc:implementation>
</html>

A using page which allows switching between hide/show this component

<h:body>
    <h:form id="testForm">

        <c:if test="#{testBean.itemVisible}">
            <test:testCmp id="test1"/>
        </c:if>
        <p/>

        <test:testCmp id="test2"/>
        <p/>

        <!-- show/hide dynamic item -->
        <h:commandLink value="Show Item">
            <f:ajax execute="@this" listener="#{testBean.onSetItemVisible}" render="@form"/> 
        </h:commandLink>
        <br/>
        <h:commandLink value="Hide Item">
            <f:ajax execute="@this" listener="#{testBean.onSetItemInvisible}" render="@form"/>
        </h:commandLink>
    </h:form>
</h:body>

When i switch between hide/show i get the exception:
"Component ID testForm:test2:text has already been found in the view"
When i don't use a composite component but a plain JSF component like e.g. <h:outputText> anything works well.

I tested with 2.2.14 and 2.3.0-m11

Affected Versions

[2.2.14, 2.3.0-m10]

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
Reported by dstrietz

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
tmiyar said:
Some more information on the issue:
It does not matter the id we set in the composite, we can remove the id completely and it will duplicate the second component, it looks as if component 2 is in the view, and by reloading the page through ajax, it does not care that the component is there already, it either throws the duplicate id error or duplicates the component.

My explanation of this is, having 2 components of type "mycomposite", the first composite with conditional rendering c:if, the c:if puts some kind of flag in the view like "render" that affects all components it finds of type "mycomposite" it finds afterwards, therefore, "mycomposite2" gets re-rendered. The "scope" of the "flag" is wrong.

If you move the c:if to the second composite, it works fine as the first composite was evaluated before the c:if sets the "flag".
Might be related to the following issues but I'm using sessionscoped
https://java.net/jira/browse/JAVASERVERFACES-1665
https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-787

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
klavikka said:
A few other users (me included) have reported this phenomenon already a couple of years ago.

See:

I'm still waiting for a fix. I tried to figure out a solution myself, but the code appeared to be a bit too complex for me to understand. Some documentation about the internals of Mojarra would have been useful, but I couldn't find anything besides the source code itself.

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
rickyepoderi said:
Hi,

I've been looking to this issue and the problem is related to how the internal IDs are calculated in mojarra. In the example:

1.- The first time the c:if is evaluated to false and only one composite is created "test2":

id:test2
       type:javax.faces.component.UINamingContainer
       value = null
       attr = javax.faces.component.BEANINFO_KEY : com.sun.faces.facelets.tag.composite.CompositeComponentBeanInfo@159b2c93
       attr = javax.faces.application.Resource.ComponentResource : /dupid/javax.faces.resource/testCmp.xhtml.xhtml?ln=test
       attr = javax.faces.component.VIEW_LOCATION_KEY : /index.xhtml @24,35
       attr = javax.faces.view.AttachedObjectHandlers : []
       attr = com.sun.faces.facelets.MARK_ID : 2059540600_7ac2180d
         id:j_id2
         type:javax.faces.component.UIPanel
         value = null
         attr = javax.faces.application.Resource.ComponentResource : something
           id:j_idt6
           type:javax.faces.component.html.HtmlOutputText
           expression/value = #{testBean.increment} : 3
           attr = javax.faces.component.VIEW_LOCATION_KEY : /resources/test/testCmp.xhtml @7,54
           attr = com.sun.faces.facelets.MARK_ID : -1950651219_36361211

2.- When you click the show link, the c:if is true and therefore two composites appear ("test1" and "test2"). The first one is:

id:test1
 type:javax.faces.component.UINamingContainer
 value = null
 attr = javax.faces.component.BEANINFO_KEY : com.sun.faces.facelets.tag.composite.CompositeComponentBeanInfo@5339e96d
 attr = javax.faces.application.Resource.ComponentResource : /dupid/javax.faces.resource/testCmp.xhtml.xhtml?ln=test
 attr = javax.faces.component.VIEW_LOCATION_KEY : /index.xhtml @19,39
 attr = com.sun.faces.facelets.MARK_ID : 2059540600_7ac21810
   id:null
   type:javax.faces.component.UIPanel
   value = null
   attr = javax.faces.application.Resource.ComponentResource : something
     id:j_idt6
     type:javax.faces.component.html.HtmlOutputText
     expression/value = #{testBean.increment} : 7
     attr = javax.faces.component.VIEW_LOCATION_KEY : /resources/test/testCmp.xhtml @7,54
     attr = com.sun.faces.facelets.MARK_ID : -1950651219_36361211

You see the MARK_ID is exactly the same as in point 1. The IDs are assigned using the composite hierarchy (only one here) and the tag. When there are more than one a number suffix is added (see later). Here because the "test1" is the first composite being evaluated it receives the same ID "test2" received in the previous request: "-1950651219_36361211".

3.- When the JSF implementation reaches the "test2" composite the HtmlOutputComponent received the ID "-1950651219_1_36361211_1" because there is already one with the same ID and the suffix is incremented (I mean, because "test1" exists, "test2" components get a "_1" suffix). But the previous value remains and it finally finishes with a duplicated component:

id:test2
 type:javax.faces.component.UINamingContainer
 value = null
 attr = javax.faces.component.BEANINFO_KEY : com.sun.faces.facelets.tag.composite.CompositeComponentBeanInfo@4ff7c157
 attr = javax.faces.application.Resource.ComponentResource : /dupid/javax.faces.resource/testCmp.xhtml.xhtml?ln=test
 attr = com.sun.faces.facelets.MARK_DELETED : true
 attr = javax.faces.component.VIEW_LOCATION_KEY : /index.xhtml @24,35
 attr = javax.faces.view.AttachedObjectHandlers : []
 attr = com.sun.faces.facelets.MARK_ID : 2059540600_7ac2180d
   id:j_id3
   type:javax.faces.component.UIPanel
   value = null
   attr = javax.faces.application.Resource.ComponentResource : something
     id:j_idt6
     type:javax.faces.component.html.HtmlOutputText
     expression/value = #{testBean.increment} : 8
     attr = javax.faces.component.VIEW_LOCATION_KEY : /resources/test/testCmp.xhtml @7,54
     attr = com.sun.faces.facelets.MARK_ID : -1950651219_36361211
     id:j_idt13
     type:javax.faces.component.html.HtmlOutputText
     expression/value = #{testBean.increment} : 9
     attr = javax.faces.component.VIEW_LOCATION_KEY : /resources/test/testCmp.xhtml @7,54
     attr = com.sun.faces.facelets.MARK_ID : -1950651219_1_36361211_1

You see that the HtmlOutputComponent is duplicated, one is the old component (the rendered in the previous request) and the other is the new one (calculated this request). The problem is related to how the component IDs are calculated and not marking to delete composite children. I have a little patch you can test and I'm going to put in a second comment.

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
rickyepoderi said:
I think that a quick fix is marking the children of a composite as "to be deleted" (because we cannot ensure that the IDs will be the same). This way the composite when is evaluated in the chain marks the children as "to be deleted". If the same ID is assigned to the child component, it is re-evaluated and the mark cleared. If the ID is different, a new component is added but the old one will continue marked as "to be deleted". Then the deletion part removes the old one successfully.

I have prepared a little change over 2.2.14 tag that does the marking and the deletion commented before. I have just test it against the mojarra automated test on version 2.2.14 and it passes all of them (but nothing more).

diff --git a/jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java b/jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java
index 271945b..d77df91 100644
--- a/jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java
+++ b/jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java
@@ -173,8 +173,9 @@ public final class ComponentSupport {
 Map<String, Object> attrs = fc.getAttributes();
 if (attrs.containsKey(MARK_DELETED)) {
     itr.remove();
-} else if (attrs.containsKey(IMPLICIT_PANEL) &&
-           !curEntry.getKey().equals(UIViewRoot.METADATA_FACET_NAME)) {
+               } else if (UIComponent.COMPOSITE_FACET_NAME.equals(curEntry.getKey()) ||
+          (attrs.containsKey(IMPLICIT_PANEL) &&
+           !curEntry.getKey().equals(UIViewRoot.METADATA_FACET_NAME))) {
     List<UIComponent> implicitPanelChildren = fc.getChildren();
     UIComponent innerChild;
     for (Iterator<UIComponent> innerItr = implicitPanelChildren.iterator();
@@ -421,14 +422,27 @@ public final class ComponentSupport {

         // mark all facets to be deleted
         if (c.getFacets().size() > 0) {
-            Collection col = c.getFacets().values();
+            Set col = c.getFacets().entrySet();
             UIComponent fc;
             for (Iterator itr = col.iterator(); itr.hasNext();) {
-fc = (UIComponent) itr.next();
+               Map.Entry entry = (Map.Entry) itr.next();
+               String facet = (String) entry.getKey();
+fc = (UIComponent) entry.getValue();
 Map<String, Object> attrs = fc.getAttributes();
 if (attrs.containsKey(MARK_CREATED)) {
     attrs.put(MARK_DELETED, Boolean.TRUE);
-} else if (attrs.containsKey(IMPLICIT_PANEL)) {
+} else if (UIComponent.COMPOSITE_FACET_NAME.equals(facet)) {
+   // mark the inner pannel components to be deleted
+   sz = fc.getChildCount();
+    if (sz > 0) {
+        UIComponent cc = null;
+        List cl = fc.getChildren();
+        while (--sz >= 0) {
+            cc = (UIComponent) cl.get(sz);
+            cc.getAttributes().put(MARK_DELETED, Boolean.TRUE);
+        }
+    }
+               } else if (attrs.containsKey(IMPLICIT_PANEL)) {
     List<UIComponent> implicitPanelChildren = fc.getChildren();
     Map<String, Object> innerAttrs = null;
     for (UIComponent cur : implicitPanelChildren) {

Please, consider this patch or a similar solution, but clearly this is a BUG cos this issue makes absolutely incompatible composites with JSTL that changes the component tree (c:if in this example).

Thanks in advance!

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
klavikka said:
Thanks Ricky!

I'll have to setup a build environment for Mojarra and try your patch ... and understand how it works.

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
zhangxinyuan said:
apply to 2.2.x

jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java | 24 +++++++++++++++++++-----
1 file changed, 19 insertions( + ), 5 deletions( - )

@ren-zhijun-oracle
Copy link
Contributor Author

@javaserverfaces Commented
This issue was imported from java.net JIRA JAVASERVERFACES-4240

@ren-zhijun-oracle
Copy link
Contributor Author

@arjantijms Commented
@xinyuan-zhang @ren-zhijun-oracle

Gave this a brief glance with @BalusC and it looks like the following check is missing for the new delete case:

 if (cc.getAttributes().containsKey(MARK_CREATED))

Was that intentional or just forgotten?

@ren-zhijun-oracle
Copy link
Contributor Author

@martin654 Commented
This issue is bothers me too. Is there any chance this could be fixed in next 2.2.xx version?

@ren-zhijun-oracle
Copy link
Contributor Author

@xinyuan-zhang Commented
@martin654 This issue had been fixed in the JSF 2.2.14 and 2.3.2

@ren-zhijun-oracle
Copy link
Contributor Author

@jensberke Commented
@xinyuan-zhang: I'm using 2.2.14 and I can still reproduce the bug there. Either it hasn't been fixed in this version or the fix doesn't work.

@ren-zhijun-oracle
Copy link
Contributor Author

@xinyuan-zhang Commented
@jensberke Hi, Can you provide a test war application for this bug?

@ren-zhijun-oracle
Copy link
Contributor Author

@martin654 Commented
@xinyuan-zhang thank you for quick reply.

I am not sure it is fixed. I have very simple app with following page and composite and it is still not working with mojarra 2.2.14 (Myfaces works)

Page:

<p:panel header="BUG" styleClass="ContainerIndent">

        <p:selectOneRadio id="text" label="Typ karty" value="#{editor.text}" columns="1" layout="grid">
          <p:ajax event="change" update="@form:panelWithIf @form:end" />
          <f:selectItems value="#{editor.texts1}" />
          <f:selectItem itemLabel="No select option" noSelectionOption="true" />
        </p:selectOneRadio>

        <h:panelGroup id="panelWithIf">
          <c:if test="#{not empty editor.text}">
            <f:subview id="foo2">
              <c:forEach items="#{editor.texts1}" var="textVar" varStatus="textVarStatus">
                <mkyong:outputText value="value #{textVar}" />
                <br />
              </c:forEach>
            </f:subview>
          </c:if>
        </h:panelGroup>
        
        <mkyong:outputText value="--- End panel group ---" />
        
        <mkyong:outputText id="end" value="value #{editor.text}" />
        
      </p:panel>

Composite:

<?xml version="1.0" encoding="UTF-8"?>
<ui:component xmlns="http://www.w3.org/1999/xhtml" xmlns:f="http://xmlns.jcp.org/jsf/core" xmlns:ui="http://xmlns.jcp.org/jsf/facelets"
  xmlns:cc="http://xmlns.jcp.org/jsf/composite" xmlns:fobosprivate="http://fobos.csob.cz/jsf/taglib/fobosprivate" xmlns:h="http://xmlns.jcp.org/jsf/html">

  <cc:interface name="outputText" shortDescription="outputTextLabeled is an extension to a standard primefces OutputLabel.">
    <cc:attribute name="value" type="java.lang.Object" required="true" />
    <cc:valueHolder name="outputText" targets="outputText" />
  </cc:interface>

  <cc:implementation>

    <div id="#{cc.clientId}" class="fob-component fob-text">


      <h:outputText id="outputText" value="#{cc.attrs.value}">
        <cc:insertChildren />
      </h:outputText>

    </div>

  </cc:implementation>
</ui:component>

Bean:

@ManagedBean(name = "editor")
public class EditorBean {
    private String text;
    private List<String> texts1 = Arrays.asList("CLASSIC S", "EXTRA S", "PREMIUM S");
}

@ren-zhijun-oracle
Copy link
Contributor Author

@jensberke Commented
@xinyuan-zhang: yes, but I've got no time for it right now. Probably tomorrow or the day after.

@ren-zhijun-oracle
Copy link
Contributor Author

@jensberke Commented
@xinyuan-zhang: I managed to find some time to create a war file, see mojarra-issue-4244.zip

@ren-zhijun-oracle
Copy link
Contributor Author

@martin654 Commented
Hi, it has been a while now, is there any progress on this issue? Thank You

@ren-zhijun-oracle
Copy link
Contributor Author

@martin654 Commented
@xinyuan-zhang: Isn't the issue fixed in this changeset?
javaserverfaces/mojarra@14e87ea

I tested version 2.2.8-22 in my example and it works. I do not see the changes from 2.2.8-22 in the 2.2.14 version.

It seems to me that it could be merged into 2.2.15...

@ren-zhijun-oracle
Copy link
Contributor Author

@arjantijms Commented
@martin654

That change set is indeed in the 2_2 rolling branch and done a good while after 2.2.14 was released, so obviously it's not in 2.2.14.

@xinyuan-zhang @ren-zhijun-oracle It's probably high time to do a 2.2.15 release cut as well. What do you think?

@ren-zhijun-oracle
Copy link
Contributor Author

@xinyuan-zhang Commented
Hi @arjantijms ,
I have just released Mojarra version 2.2.15, See
https://github.com/javaserverfaces/mojarra/releases/tag/2.2.15

Hi @martin654 ,
Mojarra 2.2.15 fixed this issue.

@ren-zhijun-oracle
Copy link
Contributor Author

@arjantijms Commented
@xinyuan-zhang I saw it, thx! :)

@ren-zhijun-oracle
Copy link
Contributor Author

@xinyuan-zhang Commented
Hi @jensberke ,
Mojarra 2.2.15 fixed this issue.

@ren-zhijun-oracle
Copy link
Contributor Author

@jensberke Commented
Hi @xinyuan-zhang. I tested it and can confirm that it's fixed in 2.2.15.

@ren-zhijun-oracle
Copy link
Contributor Author

@martin654 Commented
@xinyuan-zhang Thanks for 2.2.15 release, it works!

@ren-zhijun-oracle
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant