ICEfaces
  1. ICEfaces
  2. ICE-9398

DOM clone/copy reduces adjacent TEXT nodes which causes unintended DOM differences, so javascript executes redundantly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: EE-3.3.0.GA
    • Fix Version/s: EE-3.3.0.GA_P01, 4.0.BETA, 4.0
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      All
    • Assignee Priority:
      P1

      Description

      The framework core, compat and ACE components make multiple subsequent calls to ResponseWriter.write and ResponseWriter.writeText, to output fragments of javascript, which get rendered out as a single javascript call due to our framework concatenating them.

      Either from the DOM cloning or the FastInfoSet copying (definitely latter but possibly also former), we now have situations where the old DOM has a single TEXT node for javascript calls, while the new DOM has the multiple sibling TEXT nodes, so a DOM difference occurs. This can trigger HEAD updates, and at the very least, cause javascript to re-execute that we may have intended to not be executed beyond the first time it was rendered.

      Example of files affected that I have fixed in my workplan:

      M icefaces/compat/core/src/main/java/com/icesoft/faces/application/ExtrasSetup.java
      M icefaces/compat/core/src/main/java/com/icesoft/faces/context/CompatDOMPartialViewContext.java
      M icefaces/ace/component/src/org/icefaces/ace/component/selectmenu/SelectMenuRenderer.java
      M icefaces/ace/component/src/org/icefaces/ace/component/combobox/ComboBoxRenderer.java
      M icefaces/ace/component/src/org/icefaces/ace/component/tabset/TabSetRenderer.java
      M icefaces/ace/component/src/org/icefaces/ace/component/autocompleteentry/AutoCompleteEntryRenderer.java
      M icefaces/core/src/main/java/org/icefaces/impl/event/BridgeSetup.java
      M icefaces/core/src/main/java/org/icefaces/impl/component/NavigationNotifier.java
      M icefaces/core/src/main/java/org/icefaces/impl/context/DOMResponseWriter.java
      M icefaces/core/src/main/java/org/icefaces/impl/facelets/tag/icefaces/core/RefreshHandler.java
      M icefaces/core/src/main/java/org/icefaces/impl/renderkit/DOMRenderKit.java

        Issue Links

          Activity

          Mark Collette created issue -
          Ken Fyten made changes -
          Field Original Value New Value
          Assignee Mircea Toma [ mircea.toma ]
          Fix Version/s 3.4 [ 10770 ]
          Assignee Priority P1 [ 10010 ]
          Hide
          Mark Collette added a comment -

          The one good thing about the multiple TEXT nodes was that the memory usage before serialisation was minimised, since the Strings that were constants were preserved and isolated from the Strings that were dynamically generated (viewId, clientIds). By concatenating the TEXT nodes together, operations may be simpler and faster, but they're not interned or even internable.

          Show
          Mark Collette added a comment - The one good thing about the multiple TEXT nodes was that the memory usage before serialisation was minimised, since the Strings that were constants were preserved and isolated from the Strings that were dynamically generated (viewId, clientIds). By concatenating the TEXT nodes together, operations may be simpler and faster, but they're not interned or even internable.
          Mark Collette made changes -
          Summary DOM clone/copy reduces adjacent TEXT nodes which causes unintended DOM differences, so javascript executes redundently DOM clone/copy reduces adjacent TEXT nodes which causes unintended DOM differences, so javascript executes redundantly
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #36608 Mon Jul 08 16:47:13 MDT 2013 mircea.toma ICE-9398 Revert back to native DOM cloning but with error checking turned off.
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
          Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMResponseWriter.java
          Hide
          Mircea Toma added a comment - - edited

          Reverted back to native DOM cloning but this time the document's strict error checking is turned off. This way the cloning can complete even when the declared default namespace is not correct. ICE-6717 regression test used for replicating the DOM exception described by ICE-9379 is using http://www.w3.org/1999/xhtml as the default namespace, the expected namespace being http://www.w3.org/2000/xmlns/.
          Also, in DOMPartialViewContext.generateElementUpdateNotifications the operation's ID is used now to lookup the element to avoid ClassCastException in case the update is represented as a text node with the element's markup.

          Show
          Mircea Toma added a comment - - edited Reverted back to native DOM cloning but this time the document's strict error checking is turned off. This way the cloning can complete even when the declared default namespace is not correct. ICE-6717 regression test used for replicating the DOM exception described by ICE-9379 is using http://www.w3.org/1999/xhtml as the default namespace, the expected namespace being http://www.w3.org/2000/xmlns/ . Also, in DOMPartialViewContext.generateElementUpdateNotifications the operation's ID is used now to lookup the element to avoid ClassCastException in case the update is represented as a text node with the element's markup.
          Mircea Toma made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Ken Fyten added a comment -

          There are many instances of NullPointerException found in tomcat log - see below.

          ICEfaces3/trunk revision# 36609
          Server: tomcat6
          Browser: FF3.6, IE8, GoogleChrome27

          New failures: Many new failures, see attached Summary file.

          NullPointerExceptions are visible in tomcat log when interacting with some of the test pages; as example, ace:accordion in h:dataTable - the error occurs when changing tab panes.

          There are also errors in JS console (all browsers):
          Chrome:
          GET http://localhost:8080/accordion/RES_NOT_FOUND
          404 (Not Found) accordionDataTbl.jsf:12
          [window] Error [status: emptyResponse code:
          200]: An empty response was received from the server.
          Check server error logs. bridge-support.js.jsf:1805
          [window] Error
          [status: emptyResponse code: 200]: An empty response
          was received from the server. Check server error logs.
          bridge-support.js.jsf:1805

          IE8:
          [window] Error [status: malformedXML code: 200]: XML Parsing Error: Invalid xml declaration.
          Location:
          Line Number 2, Column 21:
          <partial-response><?xml version='1.0' encoding='UTF-8'?>
          ---------------------^

          Server error:
          Jul 9, 2013 3:32:39 AM com.sun.faces.application.view.FaceletViewHandlingStrategy handleRenderException
          SEVERE: Error Rendering View[/panelDynAttribute.xhtml]
          java.lang.NullPointerException
          at java.util.Hashtable.get(Hashtable.java:334)
          at com.sun.org.apache.xerces.internal.dom.CoreDocumentImpl.getIdentifier(CoreDocumentImpl.java:2005)
          at com.sun.org.apache.xerces.internal.dom.CoreDocumentImpl.getElementById(CoreDocumentImpl.java:1949)
          at org.icefaces.impl.context.DOMPartialViewContext.generateElementUpdateNotifications(DOMPartialViewContext.java:285)
          at org.icefaces.impl.context.DOMPartialViewContext.processPartial(DOMPartialViewContext.java:254)
          at javax.faces.component.UIViewRoot.encodeChildren(UIViewRoot.java:973)
          at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779)
          at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:413)
          at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:124)
          at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:120)
          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:298)
          at org.apache.coyote.http11.Http11AprProcessor.process(Http11AprProcessor.java:859)
          at org.apache.coyote.http11.Http11AprProtocol$Http11ConnectionHandler.process(Http11AprProtocol.java:579)
          at org.apache.tomcat.util.net.AprEndpoint$Worker.run(AprEndpoint.java:1555)
          at java.lang.Thread.run(Thread.java:619)

          Show
          Ken Fyten added a comment - There are many instances of NullPointerException found in tomcat log - see below. ICEfaces3/trunk revision# 36609 Server: tomcat6 Browser: FF3.6, IE8, GoogleChrome27 New failures: Many new failures, see attached Summary file. NullPointerExceptions are visible in tomcat log when interacting with some of the test pages; as example, ace:accordion in h:dataTable - the error occurs when changing tab panes. There are also errors in JS console (all browsers): Chrome: GET http://localhost:8080/accordion/RES_NOT_FOUND 404 (Not Found) accordionDataTbl.jsf:12 [window] Error [status: emptyResponse code: 200]: An empty response was received from the server. Check server error logs. bridge-support.js.jsf:1805 [window] Error [status: emptyResponse code: 200] : An empty response was received from the server. Check server error logs. bridge-support.js.jsf:1805 IE8: [window] Error [status: malformedXML code: 200] : XML Parsing Error: Invalid xml declaration. Location: Line Number 2, Column 21: <partial-response><?xml version='1.0' encoding='UTF-8'?> ---------------------^ Server error: Jul 9, 2013 3:32:39 AM com.sun.faces.application.view.FaceletViewHandlingStrategy handleRenderException SEVERE: Error Rendering View [/panelDynAttribute.xhtml] java.lang.NullPointerException at java.util.Hashtable.get(Hashtable.java:334) at com.sun.org.apache.xerces.internal.dom.CoreDocumentImpl.getIdentifier(CoreDocumentImpl.java:2005) at com.sun.org.apache.xerces.internal.dom.CoreDocumentImpl.getElementById(CoreDocumentImpl.java:1949) at org.icefaces.impl.context.DOMPartialViewContext.generateElementUpdateNotifications(DOMPartialViewContext.java:285) at org.icefaces.impl.context.DOMPartialViewContext.processPartial(DOMPartialViewContext.java:254) at javax.faces.component.UIViewRoot.encodeChildren(UIViewRoot.java:973) at javax.faces.component.UIComponent.encodeAll(UIComponent.java:1779) at com.sun.faces.application.view.FaceletViewHandlingStrategy.renderView(FaceletViewHandlingStrategy.java:413) at com.sun.faces.application.view.MultiViewHandler.renderView(MultiViewHandler.java:124) at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:120) 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:298) at org.apache.coyote.http11.Http11AprProcessor.process(Http11AprProcessor.java:859) at org.apache.coyote.http11.Http11AprProtocol$Http11ConnectionHandler.process(Http11AprProtocol.java:579) at org.apache.tomcat.util.net.AprEndpoint$Worker.run(AprEndpoint.java:1555) at java.lang.Thread.run(Thread.java:619)
          Ken Fyten made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Mircea Toma added a comment -

          Modified DOMPartialViewContext.generateElementUpdateNotifications method to adapt to the type of edit operation it receives. If the operation's ID is not set then operation's element ID attribute is used for looking up the updated element. When neither of methods succeed a warning message is logged.

          Show
          Mircea Toma added a comment - Modified DOMPartialViewContext.generateElementUpdateNotifications method to adapt to the type of edit operation it receives. If the operation's ID is not set then operation's element ID attribute is used for looking up the updated element. When neither of methods succeed a warning message is logged.
          Mircea Toma made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #36616 Tue Jul 09 14:28:11 MDT 2013 mircea.toma ICE-9398 Modified generateElementUpdateNotifications method to adapt to the type of edit operation it receives. If the operation's ID is not set then operation's element ID attribute is used for looking up the updated element. When neither of methods succeed a warning message is logged.
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMPartialViewContext.java
          Ken Fyten made changes -
          Fix Version/s EE-3.3.0.GA_P01 [ 11174 ]
          Ken Fyten made changes -
          Security Private [ 10001 ]
          Ken Fyten made changes -
          Link This issue blocks ICE-9404 [ ICE-9404 ]
          Hide
          Mark Collette added a comment -

          Component and framework code changes to remove most of the multiple write/writeText calls. Adding to the jira so I can revert these files from my workplan.

          Show
          Mark Collette added a comment - Component and framework code changes to remove most of the multiple write/writeText calls. Adding to the jira so I can revert these files from my workplan.
          Mark Collette made changes -
          Attachment ICE-9398.patch [ 16114 ]
          Hide
          Ken Fyten added a comment -

          Fix commits need to be ported to the EE 3.3.0.GA_P01 release tag:

          http://10.18.39.25:8888/svn/ossrepo/icefaces-ee/tags/icefaces-ee-3.3.0.GA_P01

          Show
          Ken Fyten added a comment - Fix commits need to be ported to the EE 3.3.0.GA_P01 release tag: http://10.18.39.25:8888/svn/ossrepo/icefaces-ee/tags/icefaces-ee-3.3.0.GA_P01
          Ken Fyten made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Repository Revision Date User Message
          ICEsoft Public SVN Repository #36697 Mon Jul 15 15:28:54 MDT 2013 mircea.toma ICE-9398 Make sure the imported node is added to the document and all ID-ed elements can be looked up.
          Files Changed
          Commit graph MODIFY /icefaces3/trunk/icefaces/core/src/main/java/org/icefaces/impl/context/DOMResponseWriter.java
          Hide
          Mircea Toma added a comment -

          Finished document cloning implementation by making sure that the imported node is added to the document and all ID-ed elements can be looked up.

          Show
          Mircea Toma added a comment - Finished document cloning implementation by making sure that the imported node is added to the document and all ID-ed elements can be looked up.
          Mircea Toma made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Deryk Sinotte made changes -
          Link This issue blocks ICE-9421 [ ICE-9421 ]
          Ken Fyten made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Ken Fyten made changes -
          Comment [ A comment with security level 'icesoft-internal-developers' was removed. ]
          Ken Fyten made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Ken Fyten made changes -
          Fix Version/s 4.0 [ 11382 ]
          Ken Fyten made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Mircea Toma
              Reporter:
              Mark Collette
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: