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

          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.
          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.
          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)
          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.
          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.
          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
          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.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: