ICEfaces
  1. ICEfaces
  2. ICE-7032

DOM difference handle sub-component rendering

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2
    • Fix Version/s: 2.1-Beta, 3.0
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      ICEfaces 2, ACE 2.1
    • Assignee Priority:
      P2
    • Workaround Exists:
      Yes
    • Workaround Description:
      Disable the DOM differencing of this component, or of all partial updates.

      Description

      Some of the new ACE components use a technique whereby they render a portion of themselves, instead of their full selves. For example, when sorting, the new data table will only render out the table contents, and not the headers and footers. Or, the tabView component, with dynamic="true", cache="true" and a tabChangeListener, will only render the newly visited tab contents, when they have not yet been shown, instead of the whole tabView, since it is caching the other already loaded tab contents.

      In org.icefaces.impl.context.DOMPartialRenderCallback.visit(-), around line 458, we find the position of the root of the component, by assuming the component clientId is the id of the root DOM element. We then remove the whole DOM sub-tree corresponding to that component, and setup the DOM cursor so that the component may fully render itself in the vacated space. We then search for the new elements by the clientId, so that we can compare the old sub-tree to the new sub-tree. But if the newly rendered output is not of the whole component, the search for the clientId fails, and we get a NPE.

      What needs to be done, is to allow the component to render itself first, adjacent to the old rendering of itself. And then the id of the top-most newly rendered element should be used to find the corresponding old element. If the new top-most element does not have an id, or if it's not found in the old rendering, then the update should just be sent to the browser, without any DOM differencing, and the bridge will deal with it. If the corresponding old element is found, then that old sub-tree may be scraped out, the new sub-tree supplanted in it's place, and the differencing done, so that the optimal change set can be transmitted. In most cases, the top element ids will be the clientId, and it will be the whole component being swapped in, but in special new ACE scenarios, it will be the sub-component regions.
      1. screenshot1.jpg
        91 kB
      2. screenshot2.jpg
        78 kB

        Activity

        Mark Collette created issue -
        Mark Collette made changes -
        Field Original Value New Value
        Salesforce Case []
        Description Some of the new ACE components use a technique whereby they render a portion of themselves, instead of their full selves. For example, when sorting, the new data table will only render out the table contents, and not the headers and footers. Or, the tabView component, with dynamic="true", cache="true" and a tabChangeListener, will only render the newly visited tab contents, when they have not yet been shown, instead of the whole tabView, since it is caching the other already loaded tab contents.

        In org.icefaces.impl.context.DOMPartialRenderCallback.visit(-), around line 458, we find the position of the root of the component, by assuming the component clientId is the id of the root DOM element. We then remove the whole DOM sub-tree corresponding to that component, and setup the DOM cursor so that the component may fully render itself in the vacated space. We then search for the new elements by the clientId, so that we can compare the old sub-tree to the new sub-tree. But if the newly rendered output is not of the whole component, the search for the clientId fails, and we get a NPE.

        What needs to be done, is to allow the component to render itself first, adjacent to the old rendering of itself. And then the id of the top-most newly rendered element should be used to find the corresponding old element. If the new top-most elemnet does not have an id, or if it's not found in the old rendering, then the update should just be sent to the browser, without any DOM differencing, and the bridge will deal with it. If the corresponding old element is found, then that old sub-trre may be scraped out, the new sub-tree supplanted in it's place, and the differencing done, so that the optimal change set can be transmitted. In most cases, the top element ids will be the clientId, and it will be the whole component being swapped in, but in special new ACE scenarios, it will be the sub-component regions.
        Some of the new ACE components use a technique whereby they render a portion of themselves, instead of their full selves. For example, when sorting, the new data table will only render out the table contents, and not the headers and footers. Or, the tabView component, with dynamic="true", cache="true" and a tabChangeListener, will only render the newly visited tab contents, when they have not yet been shown, instead of the whole tabView, since it is caching the other already loaded tab contents.

        In org.icefaces.impl.context.DOMPartialRenderCallback.visit(-), around line 458, we find the position of the root of the component, by assuming the component clientId is the id of the root DOM element. We then remove the whole DOM sub-tree corresponding to that component, and setup the DOM cursor so that the component may fully render itself in the vacated space. We then search for the new elements by the clientId, so that we can compare the old sub-tree to the new sub-tree. But if the newly rendered output is not of the whole component, the search for the clientId fails, and we get a NPE.

        What needs to be done, is to allow the component to render itself first, adjacent to the old rendering of itself. And then the id of the top-most newly rendered element should be used to find the corresponding old element. If the new top-most element does not have an id, or if it's not found in the old rendering, then the update should just be sent to the browser, without any DOM differencing, and the bridge will deal with it. If the corresponding old element is found, then that old sub-tree may be scraped out, the new sub-tree supplanted in it's place, and the differencing done, so that the optimal change set can be transmitted. In most cases, the top element ids will be the clientId, and it will be the whole component being swapped in, but in special new ACE scenarios, it will be the sub-component regions.
        Mark Collette made changes -
        Salesforce Case []
        Fix Version/s 2.1 [ 10241 ]
        Mark Collette made changes -
        Link This issue blocks ICE-7034 [ ICE-7034 ]
        Mark Collette made changes -
        Workaround Description Disable the DOM differencing of this component, or of all partial updates.
        Workaround Exists [Yes]
        Salesforce Case []
        Hide
        Mark Collette added a comment -

        Right now we could use the technique from ICE-6950 for TabView, to make it specifically opt out of DOM differencing for partial updates. It seems that ICE-6950 was made for 2 scenarios, the JSON response, and these sub-component responses, when maybe ICE-6950 should only be used for the JSON response issue, and this jira ICE-7032 should be for the sub-component issue.

        Show
        Mark Collette added a comment - Right now we could use the technique from ICE-6950 for TabView, to make it specifically opt out of DOM differencing for partial updates. It seems that ICE-6950 was made for 2 scenarios, the JSON response, and these sub-component responses, when maybe ICE-6950 should only be used for the JSON response issue, and this jira ICE-7032 should be for the sub-component issue.
        Hide
        Arturo Zambrano added a comment -

        Attached test case, where problem is seen again (accordionPanel.war).

        Steps to reproduce:
        1. Deploy the application.
        2. Navigate to http://localhost:8080/accordionPanel/accordionPanelAttribute.jsf
        3. Check the checkbox next to 'dynamic'.
        4. Click on any of the closed tabs in the accordion panel.
        5. The tab will not open and the following exception is thrown in the console:

        14-Sep-2011 1:58:11 PM org.icefaces.impl.util.DOMUtils nodeDiff
        SEVERE: Pruning failure
        java.lang.NullPointerException
        at org.icefaces.impl.util.DOMUtils.compareNodes(DOMUtils.java:419)
        at org.icefaces.impl.util.DOMUtils.nodeDiff(DOMUtils.java:394)
        at org.icefaces.impl.context.DOMPartialRenderCallback.visit(DOMPartialViewContext.java:513)
        at com.sun.faces.component.visit.PartialVisitContext.invokeVisitCallback(PartialVisitContext.java:183)
        at javax.faces.component.UIComponent.visitTree(UIComponent.java:1589)
        at javax.faces.component.UIComponent.visitTree(UIComponent.java:1600)
        at javax.faces.component.UIForm.visitTree(UIForm.java:344)
        at javax.faces.component.UIComponent.visitTree(UIComponent.java:1600)
        at javax.faces.component.UIComponent.visitTree(UIComponent.java:1600)
        at org.icefaces.impl.context.DOMPartialViewContext.renderSubtrees(DOMPartialViewContext.java:330)
        at org.icefaces.impl.context.DOMPartialViewContext.processPartial(DOMPartialViewContext.java:160)
        at javax.faces.component.UIViewRoot.encodeChildren(UIViewRoot.java:981)
        at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1756)
        at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:390)
        at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:131)
        at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:121)
        at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101)
        at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:139)
        at javax.faces.webapp.FacesServlet.service(FacesServlet.java:594)
        at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290)
        at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
        at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233)
        at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191)
        at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:127)
        at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102)
        at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109)
        at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:291)
        at org.apache.coyote.http11.Http11Processor.process(Http11Processor.java:859)
        at org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process(Http11Protocol.java:602)
        at org.apache.tomcat.util.net.JIoEndpoint$Worker.run(JIoEndpoint.java:489)
        at java.lang.Thread.run(Thread.java:662)

        Show
        Arturo Zambrano added a comment - Attached test case, where problem is seen again (accordionPanel.war). Steps to reproduce: 1. Deploy the application. 2. Navigate to http://localhost:8080/accordionPanel/accordionPanelAttribute.jsf 3. Check the checkbox next to 'dynamic'. 4. Click on any of the closed tabs in the accordion panel. 5. The tab will not open and the following exception is thrown in the console: 14-Sep-2011 1:58:11 PM org.icefaces.impl.util.DOMUtils nodeDiff SEVERE: Pruning failure java.lang.NullPointerException at org.icefaces.impl.util.DOMUtils.compareNodes(DOMUtils.java:419) at org.icefaces.impl.util.DOMUtils.nodeDiff(DOMUtils.java:394) at org.icefaces.impl.context.DOMPartialRenderCallback.visit(DOMPartialViewContext.java:513) at com.sun.faces.component.visit.PartialVisitContext.invokeVisitCallback(PartialVisitContext.java:183) at javax.faces.component.UIComponent.visitTree(UIComponent.java:1589) at javax.faces.component.UIComponent.visitTree(UIComponent.java:1600) at javax.faces.component.UIForm.visitTree(UIForm.java:344) at javax.faces.component.UIComponent.visitTree(UIComponent.java:1600) at javax.faces.component.UIComponent.visitTree(UIComponent.java:1600) at org.icefaces.impl.context.DOMPartialViewContext.renderSubtrees(DOMPartialViewContext.java:330) at org.icefaces.impl.context.DOMPartialViewContext.processPartial(DOMPartialViewContext.java:160) at javax.faces.component.UIViewRoot.encodeChildren(UIViewRoot.java:981) at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1756) at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:390) at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:131) at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:121) at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101) at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:139) at javax.faces.webapp.FacesServlet.service(FacesServlet.java:594) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:127) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102) at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:291) at org.apache.coyote.http11.Http11Processor.process(Http11Processor.java:859) at org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process(Http11Protocol.java:602) at org.apache.tomcat.util.net.JIoEndpoint$Worker.run(JIoEndpoint.java:489) at java.lang.Thread.run(Thread.java:662)
        Arturo Zambrano made changes -
        Attachment accordionPanel.war [ 13521 ]
        Ken Fyten made changes -
        Salesforce Case []
        Assignee Priority P1
        Assignee Ted Goddard [ ted.goddard ]
        Hide
        Ted Goddard added a comment -

        Is there anything unusual that is being written to the DOM that might cause this?

        Show
        Ted Goddard added a comment - Is there anything unusual that is being written to the DOM that might cause this?
        Ted Goddard made changes -
        Assignee Ted Goddard [ ted.goddard ] Arturo Zambrano [ artzambrano ]
        Hide
        Arturo Zambrano added a comment -

        Carmen detected this problem in her test app (attached). This problem doesn't happen in comp-suite. I wonder if it has anything to do with the fact that the 'dynamic' attribute is being set dynamically?

        Show
        Arturo Zambrano added a comment - Carmen detected this problem in her test app (attached). This problem doesn't happen in comp-suite. I wonder if it has anything to do with the fact that the 'dynamic' attribute is being set dynamically?
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #25483 Thu Sep 15 08:52:11 MDT 2011 ted.goddard handle DOM diff subtree cases where the component becomes unrendered, even though this typically indicates an application or component error (ICE-7032)
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #25484 Thu Sep 15 10:24:24 MDT 2011 ted.goddard non-Production mode will log warnings for dangerous subtree rendering cases (ICE-7032)
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
        Hide
        Ted Goddard added a comment -

        Changes have been checked in to avoid the Exception and handle the case below:

        Subtree rendering for panelForm2:panelId oldSubtree: [div: null] newSubtree: null
        Subtree rendering for panelForm2:panelId oldSubtree: null newSubtree: null

        No matter what accordion tab is clicked on, the same subtree is requested to be rendered, and it is eventually null in the DOM, meaning that no element with id "panelForm2:panelId" is found in the DOM. The corrected behavior for the DOM diff is now to produce a "delete" operation when the subtree is null. This may not be the desired effect for the component, so the component should potentially render an empty div in all cases.

        For instance, the following is acceptable since subtree rendering can add and remove components when rendered from "top":

        <h:panelGroup id="top">
        <h:panelGroup id="inner" rendered="#

        {bean.rendered}"/>
        </h:panelGroup>

        The following will work only once and then fail from then on:

        <h:panelGroup id="top" rendered="#{bean.rendered}

        ">
        <h:panelGroup id="inner" />
        </h:panelGroup>

        Once top is not rendered, it cannot be added again via subtree rendering.

        Show
        Ted Goddard added a comment - Changes have been checked in to avoid the Exception and handle the case below: Subtree rendering for panelForm2:panelId oldSubtree: [div: null] newSubtree: null Subtree rendering for panelForm2:panelId oldSubtree: null newSubtree: null No matter what accordion tab is clicked on, the same subtree is requested to be rendered, and it is eventually null in the DOM, meaning that no element with id "panelForm2:panelId" is found in the DOM. The corrected behavior for the DOM diff is now to produce a "delete" operation when the subtree is null. This may not be the desired effect for the component, so the component should potentially render an empty div in all cases. For instance, the following is acceptable since subtree rendering can add and remove components when rendered from "top": <h:panelGroup id="top"> <h:panelGroup id="inner" rendered="# {bean.rendered}"/> </h:panelGroup> The following will work only once and then fail from then on: <h:panelGroup id="top" rendered="#{bean.rendered} "> <h:panelGroup id="inner" /> </h:panelGroup> Once top is not rendered, it cannot be added again via subtree rendering.
        Hide
        Ted Goddard added a comment -

        It may make sense to add WARNING level logging for these unrecoverable subtree rendering cases.

        Show
        Ted Goddard added a comment - It may make sense to add WARNING level logging for these unrecoverable subtree rendering cases.
        Hide
        Ted Goddard added a comment -

        If the ProjectStage is not Production, the following WARNING messages will be generated for these rendering cases:

        WARNING: Subtree rendering deleting panelForm2:panelId and subsequent updates may fail.

        WARNING: Subtree rendering for panelForm2:panelId may not exist on client and replace may fail.

        So, the recommendation is to run the test in Development mode:

        <context-param>
        <param-name>javax.faces.PROJECT_STAGE</param-name>
        <param-value>Development</param-value>
        </context-param>

        and modify the component so that it does not delete the DOM subtree it will use for updating.

        Show
        Ted Goddard added a comment - If the ProjectStage is not Production, the following WARNING messages will be generated for these rendering cases: WARNING: Subtree rendering deleting panelForm2:panelId and subsequent updates may fail. WARNING: Subtree rendering for panelForm2:panelId may not exist on client and replace may fail. So, the recommendation is to run the test in Development mode: <context-param> <param-name>javax.faces.PROJECT_STAGE</param-name> <param-value>Development</param-value> </context-param> and modify the component so that it does not delete the DOM subtree it will use for updating.
        Hide
        Carmen Cristurean added a comment -

        After applying the changes to the accordionPanel test case (accordionPanel.war), there is no NullPointerException error, but the accordionPanel is not working properly after setting the attribute dynamic = true:

        • the content of the active tab is not displayed on the page (screenshot1 attached).
        • selecting any of the closed tabs will make all the tabs of the component disappear, and only the content of the selected tab will render on the page. (screenshot 2 attached). The updated .war file is also attached.
        Show
        Carmen Cristurean added a comment - After applying the changes to the accordionPanel test case (accordionPanel.war), there is no NullPointerException error, but the accordionPanel is not working properly after setting the attribute dynamic = true: the content of the active tab is not displayed on the page (screenshot1 attached). selecting any of the closed tabs will make all the tabs of the component disappear, and only the content of the selected tab will render on the page. (screenshot 2 attached). The updated .war file is also attached.
        Carmen Cristurean made changes -
        Attachment accordionPanel1.war [ 13529 ]
        Carmen Cristurean made changes -
        Attachment screenshot1.jpg [ 13530 ]
        Attachment screenshot2.jpg [ 13531 ]
        Hide
        Ted Goddard added a comment -

        This should now be fixed – the component was not indicating that it required customUpdate, causing the response to corrupt the component. (Inspecting the renderer shows that containing div elements are rendered.)

        Show
        Ted Goddard added a comment - This should now be fixed – the component was not indicating that it required customUpdate, causing the response to corrupt the component. (Inspecting the renderer shows that containing div elements are rendered.)
        Ted Goddard made changes -
        Assignee Arturo Zambrano [ artzambrano ] Carmen Cristurean [ ccristurean ]
        Hide
        Ken Fyten added a comment -

        To verify that this feature is working as expected we should create a new ace:dataTable regression test:

        Make test for dataTable row selection that will result in a custom update extension in the response, as well as use the onRowSelectUpdate to specify a region of the page to get a regular jsf update (make sure it changes so dom diff won't suppress), and also make the server side listener use JavascriptRunner to make the response also have an eval section. This way we can test all 3 types of things in the response.

        Show
        Ken Fyten added a comment - To verify that this feature is working as expected we should create a new ace:dataTable regression test: Make test for dataTable row selection that will result in a custom update extension in the response, as well as use the onRowSelectUpdate to specify a region of the page to get a regular jsf update (make sure it changes so dom diff won't suppress), and also make the server side listener use JavascriptRunner to make the response also have an eval section. This way we can test all 3 types of things in the response.
        Ken Fyten made changes -
        Assignee Carmen Cristurean [ ccristurean ] Nils Lundquist [ nils.lundquist ]
        Ken Fyten made changes -
        Salesforce Case []
        Assignee Priority P1 P2
        Hide
        Carmen Cristurean added a comment -

        These changes have been tested for the ace accordionPanel component using code Rev# 25567, on tomcat6, and this works well in Firefox 3.6, Firefox 6 and Gchrome 14.

        In IE8 browser, the dynamic attribute still doesn't work: after setting dynamic=true, when clicking on one of the closed tabs, the accordionPanel component disappears from the page.

        Show
        Carmen Cristurean added a comment - These changes have been tested for the ace accordionPanel component using code Rev# 25567, on tomcat6, and this works well in Firefox 3.6, Firefox 6 and Gchrome 14. In IE8 browser, the dynamic attribute still doesn't work: after setting dynamic=true, when clicking on one of the closed tabs, the accordionPanel component disappears from the page.
        Hide
        Nils Lundquist added a comment -

        Completed test. Works as expected.

        Show
        Nils Lundquist added a comment - Completed test. Works as expected.
        Nils Lundquist made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Carmen Cristurean added a comment -

        Tested again the ace accordionPanel - dynamic attribute using code rev#25636, and found the issue described above has been fixed.
        This has been verified on browsers: IE8, IE7, Firefox 3.6, Gchrome 14, using tomcat6 as server.

        Show
        Carmen Cristurean added a comment - Tested again the ace accordionPanel - dynamic attribute using code rev#25636, and found the issue described above has been fixed. This has been verified on browsers: IE8, IE7, Firefox 3.6, Gchrome 14, using tomcat6 as server.
        Ken Fyten made changes -
        Fix Version/s 2.1-Beta [ 10291 ]
        Ken Fyten made changes -
        Salesforce Case []
        Security Private [ 10001 ]
        Ken Fyten made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Nils Lundquist
            Reporter:
            Mark Collette
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: