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.

[bleathem]

Nice work @mpetrov.  Let's wait and see how the Mojarra team responds to your proposed fix, then pursue any specific RF fixes as required.

Brian