Bauke Scholtz created MYFACES-4775:
--------------------------------------
Summary: c:forEach writes a submitted value into the element saved
with the view state instead of into the live item
Key: MYFACES-4775
URL: https://issues.apache.org/jira/browse/MYFACES-4775
Project: MyFaces Core
Issue Type: Bug
Affects Versions: 4.1.4, 5.0.0
Reporter: Bauke Scholtz
While analyzing what the Faces view state contains, inputs bound to a property
of the {{var}} of a {{<c:forEach>}} turned out to lose their submitted value
without any message, also when nothing changes between the requests.
h3. Reproducer
{code:xml}
<h:form id="viewForm">
<c:forEach items="#{viewItems.list}" var="item">
<h:inputText id="name_#{item.id}" value="#{item.name}" />
</c:forEach>
<h:commandButton id="submit" value="Submit" />
</h:form>
<h:outputText value="Bean values: #{viewItems.list[0].name},
#{viewItems.list[1].name}" />
<h:form id="requestForm">
<c:forEach items="#{requestItems.list}" var="item">
<h:inputText id="name_#{item.id}" value="#{item.name}" />
</c:forEach>
<h:commandButton id="submit" value="Submit" />
</h:form>
<h:outputText value="Bean values: #{requestItems.list[0].name},
#{requestItems.list[1].name}" />
{code}
{code:java}
public class Item implements Serializable {
private final String id;
private String name;
// Constructor, getters and setter.
}
@Named @ViewScoped
public class ViewItems implements Serializable {
private List<Item> list;
@PostConstruct
public void init() {
list = new ArrayList<>(List.of(new Item("a", "original a"), new
Item("b", "original b")));
}
public List<Item> getList() {
return list;
}
}
@Named @RequestScoped
public class RequestItems {
private List<Item> list;
@PostConstruct
public void init() {
list = List.of(new Item("a", "original a"), new Item("b", "original
b"));
}
public List<Item> getList() {
return list;
}
}
{code}
Open the page, change both names of a form to "edited a" and "edited b", and
submit it. Nothing changes between the requests, so every build makes the same
decisions. The response renders the names from the bean:
||Items in||MyFaces 4.1.4, server||MyFaces 4.1.4, client||Mojarra 4.1.20,
server||Mojarra 4.1.20, client||
|{{@ViewScoped}} bean|edited a, edited b|*original a, original b*|edited a,
edited b|edited a, edited b|
|{{@RequestScoped}} bean|*original a, original b*|*original a, original
b*|edited a, edited b|edited a, edited b|
MyFaces 4.1.3 gives the same result. The same inputs in a {{<ui:repeat>}}
instead of a {{<c:forEach>}} receive the values in all cases.
h3. Cause
{{ForEachHandler.getVarExpr()}} binds the {{var}} of each row to the element
itself, with {{new IteratedValueExpression(ve, value)}}, also for a {{List}}
and an array. {{setVar()}} then stores that expression in the {{FaceletState}}
via {{putBinding()}}, so the element is saved with the view state. On the
postback, {{#\{item.name\}}} resolves against that saved element:
* With client side state saving, it is always a deserialized copy.
* With server side state saving, it is a copy as soon as the bean holds new
instances, as a {{@RequestScoped}} bean does on every request, or a bean which
loads the items from a database.
The submitted value then lands in an object which nobody refers to anymore.
h3. Specification
MYFACES-4029 and MYFACES-4318 report the same behavior. Both were closed as not
a bug, because {{<c:forEach>}} "just does not play well with JSF lifecycle".
That was a fair reading of the JSF 2.x specification, which indeed did not
clearly specify how the rows of a build time iteration are restored.
The Faces 5.0 specification, which is in progress, specifies it since
[jakartaee/faces#2234|https://github.com/jakartaee/faces/issues/2234]. The
javadoc of {{StateManagementStrategy.restoreView()}} sets the contract:
bq. This method returns the view that was saved: restoring the {{Object}} that
{{saveView}} returned for a view must produce that view again, holding the same
components at the same positions with the same state.
In short, the spec now finally requires {{restore(save(view))}} to equal
exactly {{view}}.
The rows of a {{<c:forEach>}} are reproduced from the build time decisions
saved with the view, such as the range and the row count. The elements are not
part of the view: each row reads its element from the items. Restore View, in
the description of build time conditions:
bq. The items an iteration was rendered over are a further exception, since its
rows read their element from those items rather than from the state: where the
items no longer hold them, the value submitted for such a row reaches nothing
that outlives the request.
So the elements themselves are not needed to restore the view, and the
reproducer above is a valid page. The javadoc of {{restoreView()}} calls the
case where the items no longer hold the elements "the case which cannot be
reproduced from saved state at all".
The Faces 5.0 TCK covers that case in
{{Issue5961IT.testIterationOverVanishedItems()}}: the rows are restored, but
the submitted values must not reach the model. With server side state saving,
MyFaces 4.1.5-SNAPSHOT fails that test: the saved element is then the same
session scoped instance, so the submitted values reach the model, and the names
read "first,second" instead of "a,b". With client side state saving, it passes
only because the values land in a deserialized copy. This was run with the page
and beans of that test deployed on Tomcat, not with the TCK harness.
Mojarra implements this since 4.0.23 and 4.1.14
([eclipse-ee4j/mojarra#5965|https://github.com/eclipse-ee4j/mojarra/pull/5965]),
with {{com.sun.faces.restoreBuildTimeDecisions}} set to {{true}}. Each row
then reads its element live, by position or by key. A row whose element is gone
reads a stand-in, and [{{ForEachHandler}} logs a
warning|https://github.com/eclipse-ee4j/mojarra/blob/f4d595f502b864b3e57bff6c6ec93564a28a7f07/impl/src/main/java/com/sun/faces/facelets/tag/jstl/core/ForEachHandler.java#L453-L455]
which names the {{<c:forEach>}}. On the page of that TCK test, Mojarra
4.1.21-SNAPSHOT passes and logs:
{noformat}
WARNING: The build which restored this postback reproduced 2 of the 2 rows
/issue5961-vanisheditems.xhtml @10,51 <c:forEach> rendered over items which no
longer hold their elements, so a value submitted for such a row is decoded into
a stand-in for the element that is gone rather than into the model.
{noformat}
h3. Proposed fix
Let a row read its element from the live items, by index for a {{List}} and an
array, as {{IndexedValueExpression(ve, i)}} did before MYFACES-3811. Keep the
saved state only for what decides the restored rows, such as the range and the
row count, and not for the elements themselves.
{{LegacyForEachHandler}}, enabled with
{{org.apache.myfaces.STRICT_JSF_2_FACELETS_COMPATIBILITY=true}}, already binds
the {{var}} of a {{List}} with {{new IndexedValueExpression(ve, i)}} and does
not put it in the {{FaceletState}}. With that handler, the reproducer above
writes "edited a, edited b" into both beans, with server and client side state
saving alike. That only shows the approach works in MyFaces: the context param
also affects the scope of {{<c:set>}} and {{<ui:param>}} across inclusions (see
[jakartaee/faces#2248|https://github.com/jakartaee/faces/issues/2248]), and the
{{<c:forEach>}} fix must apply by default, independent of any context param.
Fix is needed in MyFaces 5.0.0 in order to pass Faces 5.0 TCK and just a nice
to have in MyFaces 4.1.5.
h3. History
* MYFACES-3811 ([a2b11c073|https://github.com/apache/myfaces/commit/a2b11c073])
replaced {{new IndexedValueExpression(ve, i)}} by {{new
IteratedValueExpression(ve, value)}}. The old line is still there as a comment.
* MYFACES-3825 ([2d351df95|https://github.com/apache/myfaces/commit/2d351df95])
put the {{var}} expression in the {{FaceletState}}, which saves it with the
view state.
🤖 Generated with Claude Opus 5.5
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
