You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Michael Concini (JIRA)" <de...@myfaces.apache.org> on 2010/09/02 21:11:53 UTC

[jira] Created: (MYFACES-2908) UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing

UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing
----------------------------------------------------------------------------------------------

                 Key: MYFACES-2908
                 URL: https://issues.apache.org/jira/browse/MYFACES-2908
             Project: MyFaces Core
          Issue Type: Bug
          Components: JSR-314
    Affects Versions: 2.0.2-SNAPSHOT
            Reporter: Michael Concini
            Assignee: Michael Concini


Looks like when MYFACES-2854 was committed, there were two issues it caused where we break spec compliance.  Both are around behavior that "If the component ID of componentResource matches the the ID of a resource that has allready been added, remove the old resource."

The first is that it looks like the "else if (componentId != null)" statement is in the wrong place.  
If a UIComponent is added with the same ID as an existing component, we'll never get to this check since we'll  be false on the isInView() check.  
This will cause duplicate ID exceptions to be thrown if multiple objects with the same ID are added.  
This is easily resolved by moving the else if to kick in whenever isInView() is false.

The second issue, and the more important since this is breaking a TCK test, is that since we were only comparing to the parent's ID to the location prefix, we weren't handling the case where the same object gets added a second time after updating the target.  
This can be resolved by changing 

            if (componentResource.getParent() != null &&
                componentResource.getParent().getId() != null &&
                componentResource.getParent().getId().startsWith(JAVAX_FACES_LOCATION_PREFIX)) ...
to
            if (componentResource.getParent() != null &&
                componentResource.getParent().getId() != null &&
                componentResource.getParent().getId().equals(JAVAX_FACES_LOCATION_PREFIX + target))

I'll attach a patch for review while I wait for our CTS team to try the change out.  I probably wont' be able to commit until after the US Labor Day holiday since I'm away Friday. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (MYFACES-2908) UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/MYFACES-2908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Leonardo Uribe resolved MYFACES-2908.
-------------------------------------

    Resolution: Fixed

> UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing
> ----------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2908
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2908
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.2-SNAPSHOT
>            Reporter: Michael Concini
>            Assignee: Michael Concini
>             Fix For: 2.0.2-SNAPSHOT
>
>         Attachments: MYFACES-2908-patch.txt
>
>
> Looks like when MYFACES-2854 was committed, there were two issues it caused where we break spec compliance.  Both are around behavior that "If the component ID of componentResource matches the the ID of a resource that has allready been added, remove the old resource."
> The first is that it looks like the "else if (componentId != null)" statement is in the wrong place.  
> If a UIComponent is added with the same ID as an existing component, we'll never get to this check since we'll  be false on the isInView() check.  
> This will cause duplicate ID exceptions to be thrown if multiple objects with the same ID are added.  
> This is easily resolved by moving the else if to kick in whenever isInView() is false.
> The second issue, and the more important since this is breaking a TCK test, is that since we were only comparing to the parent's ID to the location prefix, we weren't handling the case where the same object gets added a second time after updating the target.  
> This can be resolved by changing 
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().startsWith(JAVAX_FACES_LOCATION_PREFIX)) ...
> to
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().equals(JAVAX_FACES_LOCATION_PREFIX + target))
> I'll attach a patch for review while I wait for our CTS team to try the change out.  I probably wont' be able to commit until after the US Labor Day holiday since I'm away Friday. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Reopened: (MYFACES-2908) UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing

Posted by "Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/MYFACES-2908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Leonardo Uribe reopened MYFACES-2908:
-------------------------------------


The code that check for a duplicate component is on the right place.

It seems the previous algorithm takes into consideration facelets added components but not programatically added ones. So the fix is correct if a component is added programatically but makes facelets added component (h:outputScript, h:outputStylesheet). Right now, we have tag handlers for h:outputScript and h:outputStylesheet that prevents double addition, but if some user create a tag with a similar behavior it will fail.

> UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing
> ----------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2908
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2908
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.2-SNAPSHOT
>            Reporter: Michael Concini
>            Assignee: Michael Concini
>             Fix For: 2.0.2-SNAPSHOT
>
>         Attachments: MYFACES-2908-patch.txt
>
>
> Looks like when MYFACES-2854 was committed, there were two issues it caused where we break spec compliance.  Both are around behavior that "If the component ID of componentResource matches the the ID of a resource that has allready been added, remove the old resource."
> The first is that it looks like the "else if (componentId != null)" statement is in the wrong place.  
> If a UIComponent is added with the same ID as an existing component, we'll never get to this check since we'll  be false on the isInView() check.  
> This will cause duplicate ID exceptions to be thrown if multiple objects with the same ID are added.  
> This is easily resolved by moving the else if to kick in whenever isInView() is false.
> The second issue, and the more important since this is breaking a TCK test, is that since we were only comparing to the parent's ID to the location prefix, we weren't handling the case where the same object gets added a second time after updating the target.  
> This can be resolved by changing 
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().startsWith(JAVAX_FACES_LOCATION_PREFIX)) ...
> to
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().equals(JAVAX_FACES_LOCATION_PREFIX + target))
> I'll attach a patch for review while I wait for our CTS team to try the change out.  I probably wont' be able to commit until after the US Labor Day holiday since I'm away Friday. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (MYFACES-2908) UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing

Posted by "Michael Concini (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/MYFACES-2908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Concini updated MYFACES-2908:
-------------------------------------

           Status: Resolved  (was: Patch Available)
    Fix Version/s: 2.0.2-SNAPSHOT
       Resolution: Fixed

> UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing
> ----------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2908
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2908
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.2-SNAPSHOT
>            Reporter: Michael Concini
>            Assignee: Michael Concini
>             Fix For: 2.0.2-SNAPSHOT
>
>         Attachments: MYFACES-2908-patch.txt
>
>
> Looks like when MYFACES-2854 was committed, there were two issues it caused where we break spec compliance.  Both are around behavior that "If the component ID of componentResource matches the the ID of a resource that has allready been added, remove the old resource."
> The first is that it looks like the "else if (componentId != null)" statement is in the wrong place.  
> If a UIComponent is added with the same ID as an existing component, we'll never get to this check since we'll  be false on the isInView() check.  
> This will cause duplicate ID exceptions to be thrown if multiple objects with the same ID are added.  
> This is easily resolved by moving the else if to kick in whenever isInView() is false.
> The second issue, and the more important since this is breaking a TCK test, is that since we were only comparing to the parent's ID to the location prefix, we weren't handling the case where the same object gets added a second time after updating the target.  
> This can be resolved by changing 
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().startsWith(JAVAX_FACES_LOCATION_PREFIX)) ...
> to
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().equals(JAVAX_FACES_LOCATION_PREFIX + target))
> I'll attach a patch for review while I wait for our CTS team to try the change out.  I probably wont' be able to commit until after the US Labor Day holiday since I'm away Friday. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (MYFACES-2908) UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing

Posted by "Michael Concini (JIRA)" <de...@myfaces.apache.org>.
     [ https://issues.apache.org/jira/browse/MYFACES-2908?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Concini updated MYFACES-2908:
-------------------------------------

    Status: Patch Available  (was: Open)

> UIViewRoot.addComponentResource() adding multiple components with same ID instead of replacing
> ----------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2908
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2908
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.2-SNAPSHOT
>            Reporter: Michael Concini
>            Assignee: Michael Concini
>         Attachments: MYFACES-2908-patch.txt
>
>
> Looks like when MYFACES-2854 was committed, there were two issues it caused where we break spec compliance.  Both are around behavior that "If the component ID of componentResource matches the the ID of a resource that has allready been added, remove the old resource."
> The first is that it looks like the "else if (componentId != null)" statement is in the wrong place.  
> If a UIComponent is added with the same ID as an existing component, we'll never get to this check since we'll  be false on the isInView() check.  
> This will cause duplicate ID exceptions to be thrown if multiple objects with the same ID are added.  
> This is easily resolved by moving the else if to kick in whenever isInView() is false.
> The second issue, and the more important since this is breaking a TCK test, is that since we were only comparing to the parent's ID to the location prefix, we weren't handling the case where the same object gets added a second time after updating the target.  
> This can be resolved by changing 
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().startsWith(JAVAX_FACES_LOCATION_PREFIX)) ...
> to
>             if (componentResource.getParent() != null &&
>                 componentResource.getParent().getId() != null &&
>                 componentResource.getParent().getId().equals(JAVAX_FACES_LOCATION_PREFIX + target))
> I'll attach a patch for review while I wait for our CTS team to try the change out.  I probably wont' be able to commit until after the US Labor Day holiday since I'm away Friday. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.