Fix for ui:repeat
michpetrov Apr 9, 2015 8:56 AMAs 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:
- clone my Mojarra fork and checkout "2.2.10x-jbossorg-1"
- rename build.properties.glassfish to build.properties, open it and set "jsf.build.home"
- go to jsf-ri and run "ant"
- if you want to put the jar in your local repo run "ant mvn.deploy.snapshot.local"
- The artifact is then available as org.glassfish:javax.faces:2.2.10x-SNAPSHOT
- if you want to get jsf-impl.jar go to the root folder and run "ant split-impl"
- 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.