1 Reply Latest reply on Apr 14, 2015 10:35 AM by bleathem

    Fix for ui:repeat

    michpetrov

      As mentioned on the meeting I've been working on a fix for the ui:repeat issue.

       

      To re-iterate, this is the problematic method in Mojarra 2.2.5:

      // Tests whether we need to visit our children as part of
      // a tree visit
      private boolean doVisitChildren(VisitContext context) {
      
          // Just need to check whether there are any ids under this
          // subtree.  Make sure row index is cleared out since
          // getSubtreeIdsToVisit() needs our row-less client id.
          setIndex(context.getFacesContext(), -1);
          Collection<String> idsToVisit = context.getSubtreeIdsToVisit(this);
          assert(idsToVisit != null);
      
          // All ids or non-empty collection means we need to visit our children.
          return (!idsToVisit.isEmpty());
      }
      

       

      JSF-3152 (2.2.6+) changed it to this:

      // Tests whether we need to visit our children as part of
      // a tree visit
      private boolean doVisitChildren(VisitContext context) {
      
          // Just need to check whether there are any ids under this
          // subtree.  Make sure row index is cleared out since
          // getSubtreeIdsToVisit() needs our row-less client id.
          //
          // We only need to position if row iteration is actually needed.
          //
          if (requiresRowIteration(context)) {
              setIndex(context.getFacesContext(), -1);
          }
          Collection<String> idsToVisit = context.getSubtreeIdsToVisit(this);
          assert(idsToVisit != null);
      
          // All ids or non-empty collection means we need to visit our children.
          return (!idsToVisit.isEmpty());
      }
      

       

      without the index being set to -1 the component state of the current row isn't saved and retrieved properly, causing our current issues.

      private boolean requiresRowIteration(VisitContext ctx) {
          return !ctx.getHints().contains(VisitHint.SKIP_ITERATION);
      }
      

       

      My idea is to change the method to this:

      private boolean requiresRowIteration(VisitContext ctx) {
          boolean shouldIterate = !ctx.getHints().contains(VisitHint.SKIP_ITERATION);
          FacesContext faces = ctx.getFacesContext();
          String sourceId = faces.getExternalContext().getRequestParameterMap().get("javax.faces.source");
          boolean containsSource = sourceId != null ? sourceId.startsWith(super.getClientId(faces) + getSeparatorChar(faces)): false;
         
          return (shouldIterate || containsSource);
      }
      

       

      This would ensure the rows are visited when the source of the request is inside the ui:repeat, and it wouldn't break JSF-3152 since that only deals with requests from outside the ui:repeat.

       

      I am not sure if there is a better way to tell when the source is inside the component, visitTree cannot be used since it circles back to requiresRowIteration(), and even if it did not we cannot allow a visit to the component if the source isn't there (i.e. it would break JSF-3152); invokeOnComponent cannot be used either, since that requires the index to be set to -1 in the first place.

       

      If anyone wants to test this here's a build guide:

      1. clone my Mojarra fork and checkout "2.2.10x-jbossorg-1"
      2. rename build.properties.glassfish to build.properties, open it and set "jsf.build.home"
      3. go to jsf-ri and run "ant"
      4. if you want to put the jar in your local repo run "ant mvn.deploy.snapshot.local"
        1. The artifact is then available as org.glassfish:javax.faces:2.2.10x-SNAPSHOT
      5. if you want to get jsf-impl.jar go to the root folder and run "ant split-impl"
        1. The jar will be in jsf-ri/build/lib, you can use to replace the module in WildFly

       

      I've checked all our reported issues - most of them work with this fix, the ones that do not work fail because the ajax request doesn't come from inside the ui:repeat.