ICEfaces
  1. ICEfaces
  2. ICE-9261

ace:tree - error on update following back button return

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: EE-3.3.0.GA, 4.0.BETA, 4.0
    • Component/s: ACE-Components
    • Labels:
      None
    • Environment:
      ICEfaces 3, ace:tree, single select mode
    • Assignee Priority:
      P1
    • Workaround Description:
      Hide
      @ManagedBean(eager = true)
      @ApplicationScoped
      public class HeaderPhaseListener implements Serializable {
          @PostConstruct
          private void initialize() {
              LifecycleFactory factory = (LifecycleFactory) FactoryFinder.getFactory(FactoryFinder.LIFECYCLE_FACTORY);
              Lifecycle lifecycle = factory.getLifecycle(LifecycleFactory.DEFAULT_LIFECYCLE);
              lifecycle.addPhaseListener(new PhaseListener() {
                  public void afterPhase(PhaseEvent phaseEvent) {
                      //To change body of implemented methods use File | Settings | File Templates.
                  }

                  public void beforePhase(PhaseEvent phaseEvent) {
                      ExternalContext ec = phaseEvent.getFacesContext().getExternalContext();
                      ec.addResponseHeader("Cache-Control", "no-cache, no-store, must-revalidate");
                      ec.addResponseHeader("Pragma", "no-cache");
                      ec.addResponseHeader("Expires", "0");
                  }

                  public PhaseId getPhaseId() {
                      return PhaseId.RENDER_RESPONSE;
                  }
              });
          }
      }
      Show
      @ManagedBean(eager = true) @ApplicationScoped public class HeaderPhaseListener implements Serializable {     @PostConstruct     private void initialize() {         LifecycleFactory factory = (LifecycleFactory) FactoryFinder.getFactory(FactoryFinder.LIFECYCLE_FACTORY);         Lifecycle lifecycle = factory.getLifecycle(LifecycleFactory.DEFAULT_LIFECYCLE);         lifecycle.addPhaseListener(new PhaseListener() {             public void afterPhase(PhaseEvent phaseEvent) {                 //To change body of implemented methods use File | Settings | File Templates.             }             public void beforePhase(PhaseEvent phaseEvent) {                 ExternalContext ec = phaseEvent.getFacesContext().getExternalContext();                 ec.addResponseHeader("Cache-Control", "no-cache, no-store, must-revalidate");                 ec.addResponseHeader("Pragma", "no-cache");                 ec.addResponseHeader("Expires", "0");             }             public PhaseId getPhaseId() {                 return PhaseId.RENDER_RESPONSE;             }         });     } }

      Description

      QA reports the following issue with showcase demo:

      Breadcrumbs:

      > Overview: JS error occurs in the following scenario:
      click on the '+' sign to access cities for a province.
      in 'Crumbs with URL's' click on the province name -> user is redirected to the wiki page for that province.
      click the Back button in the browser to navigate back to showcase ace:breadcrumbs - Overview page.
      select a different province from the breadcrumb menu items -> a JS error is visible in the console:
      [window] Error [status: malformedXML code: 200]: During update: crumb-form:crumb-tree::0:2:0: not found
      http://localhost:8080/showcase/javax.faces.resource/coalesced.js.jsf?ln=ice.core&dgst=4twnyr
      Line 436

      Yip's analysis:

      Nothing to do with breadcrumbs component. The tree demo has the same problem. See video. Seems like some states or some node ids (the digits in the id is the key to the tree node) are lost once you navigate elsewhere and come back. And it seems to occur only in single select mode. (Tree Overview demo uses multiple select mode, change source to use single select mode to reproduce problem.)

        Activity

        Hide
        Nils Lundquist added a comment -

        This is an interesting error - when the page is refreshed instead of a navigation being performed you'd expect the behavior to be the same. The page is being reloaded fully. However when a refresh is done instead of navigation and return, the correct expansion and selection state for the Tree is shown. When a navigation is performed the previous expansion state for the tree is not rendered.

        Show
        Nils Lundquist added a comment - This is an interesting error - when the page is refreshed instead of a navigation being performed you'd expect the behavior to be the same. The page is being reloaded fully. However when a refresh is done instead of navigation and return, the correct expansion and selection state for the Tree is shown. When a navigation is performed the previous expansion state for the tree is not rendered.
        Hide
        Nils Lundquist added a comment -

        This error is due to interaction between DOM diff and the history caching behavior of the browser when the the following HTTP headers aren't set:
        Cache-Control: no-cache, no-store, must-revalidate
        Pragma: no-cache
        Expires: 0

        When the browser returns to a page using the back button and these headers haven't been set, rather than reloading the page the browser restores the DOM it received as a response to the original GET request. This DOM doesn't contain the modifications that have been made by Ajax updates, and thus doesn't match the expected client DOM held by DOM diff.

        In this particular case, when a user returns to the original DOM using the back button, and attempts to click a node, it causes the currently selected node to now render as deselected. Since the user was looking at the original DOM however, that selected node was not shown (since it's was originally a collapsed child). DOM diff believes that selected node to exist in the client DOM, so the diff'd update specifically tries to change the selection state of the child node that is missing from the client DOM, causing the JS error reported.

        The reason that multiple selection masked this issue is that bugged selection request would not attempt to change the selection state of the previously selected node - hiding the mismatch between the original GET state now shown and the state that was navigated away from.

        Show
        Nils Lundquist added a comment - This error is due to interaction between DOM diff and the history caching behavior of the browser when the the following HTTP headers aren't set: Cache-Control: no-cache, no-store, must-revalidate Pragma: no-cache Expires: 0 When the browser returns to a page using the back button and these headers haven't been set, rather than reloading the page the browser restores the DOM it received as a response to the original GET request. This DOM doesn't contain the modifications that have been made by Ajax updates, and thus doesn't match the expected client DOM held by DOM diff. In this particular case, when a user returns to the original DOM using the back button, and attempts to click a node, it causes the currently selected node to now render as deselected. Since the user was looking at the original DOM however, that selected node was not shown (since it's was originally a collapsed child). DOM diff believes that selected node to exist in the client DOM, so the diff'd update specifically tries to change the selection state of the child node that is missing from the client DOM, causing the JS error reported. The reason that multiple selection masked this issue is that bugged selection request would not attempt to change the selection state of the previously selected node - hiding the mismatch between the original GET state now shown and the state that was navigated away from.
        Hide
        Nils Lundquist added a comment -

        Added work around PhaseListener to add headers that prevent page caching.

        Show
        Nils Lundquist added a comment - Added work around PhaseListener to add headers that prevent page caching.
        Hide
        Nils Lundquist added a comment -

        As per Ted's recommendation - should no Mojarra config option exist for this, the above PhaseListener should be added to core.

        Show
        Nils Lundquist added a comment - As per Ted's recommendation - should no Mojarra config option exist for this, the above PhaseListener should be added to core.
        Hide
        Nils Lundquist added a comment -

        No Mojarra option exists for the header in this way so I will be moving the PhaseListener to core.

        There is a remaining JS error that can be generated by navigating away, returning, and attempting to expand the subtree that was previously expanded, but this appears to be an interaction between DOM diff and Window state (where the NodeStateMap is bound) and should be handled in a new JIRA.

        Show
        Nils Lundquist added a comment - No Mojarra option exists for the header in this way so I will be moving the PhaseListener to core. There is a remaining JS error that can be generated by navigating away, returning, and attempting to expand the subtree that was previously expanded, but this appears to be an interaction between DOM diff and Window state (where the NodeStateMap is bound) and should be handled in a new JIRA.
        Hide
        Nils Lundquist added a comment -

        Revision #35532
        Committed by nils.lundquist
        6 minutes ago
        ICE-9261 - Added PhaseListener to prevent browser from caching initial GET for use in browser history. Using initial stale GET on history return can result in unpredictable updates.

        Show
        Nils Lundquist added a comment - Revision #35532 Committed by nils.lundquist 6 minutes ago ICE-9261 - Added PhaseListener to prevent browser from caching initial GET for use in browser history. Using initial stale GET on history return can result in unpredictable updates.
        Hide
        Liana Munroe added a comment -

        Confirmed fixed revision#35690 FF 21, Chrome 27, IE 9.

        Show
        Liana Munroe added a comment - Confirmed fixed revision#35690 FF 21, Chrome 27, IE 9.

          People

          • Assignee:
            Mircea Toma
            Reporter:
            Ken Fyten
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: