ResetValues: UIViewRoot#resetValues() Must walk all children components and reset

[melloware]

Original Report: https://github.com/primefaces/primefaces/issues/12241

Description:

Currently when called resetValues with say a render=":frmTest" or render="@form" it does not reset all the values of the form only the form itself is checked.

Both Mojarra and MyFaces use similar code which is like below.

 public void resetValues(FacesContext context, java.util.Collection<java.lang.String> clientIds)    
    {
        VisitContext visitContext = VisitContext.createVisitContext(context, clientIds, null);
        this.visitTree(visitContext, new VisitCallback {
                @Override
                public VisitResult visit(VisitContext context, UIComponent target)
                {
                    if (target instanceof EditableValueHolder)
                    {
                        ((EditableValueHolder)target).resetValue();
                    }
                    return VisitResult.ACCEPT;
                }
            }
    }

When called with resetValues(context, Arrays.asList("form")) it checks the first component which is an HtmlForm and then that is it. It does not recurse through the form's children to reset all values.

Additional Context

PrimeFaces used to have custom code that was working and reset the whole form but in PF 14 we removed it to rely on JSF's implementation.

[tandraschko]

@BalusC @arjantijms

AFAIR there is a difference when creating a VisitContext with ids and do:

root.visitTree(...);

or first resolve the component for the id, creating a VisitContext without ids and do:

form.visitTree(...);

the first doesnt visit childs, the second does? cant remember

[BalusC]

Agreed. The OmniFaces ResetInputAjaxActionListener also does this. The impl should boil down to this

public void resetValues(FacesContext facesContext, java.util.Collection<java.lang.String> clientIds)    
{
    VisitContext visitContext = VisitContext.createVisitContext(facesContext, clientIds, null);
    this.visitTree(visitContext, new VisitCallback()
    {
        @Override
        public VisitResult visit(VisitContext context, UIComponent target)
        {
            if (target instanceof EditableValueHolder)
            {
                ((EditableValueHolder)target).resetValue();
            }
            else if (!VisitContext.ALL_IDS.equals(context.getIdsToVisit()))
            {
                // Render ID didn't specifically point an EditableValueHolder. Visit all children as well.
                target.visitTree(VisitContext.createVisitContext(facesContext, null, context.getHints()), this);
            }

            return VisitResult.ACCEPT;
        }
    });
}

The resetValues() spec should probably be more specific in this.

Current:

     * <p class="changed_added_2_2">
     * Visit the clientIds and, if the component is an instance of {@link EditableValueHolder}, call its
     * {@link EditableValueHolder#resetValue} method. Use {@link #visitTree} to do the visiting.
     * </p>

Proposed:

     * <p class="changed_added_2_2 changed_modified_5_0">
     * Use {@link #visitTree} to visit the clientIds and, if the node is an instance of {@link EditableValueHolder}, call its
     * {@link EditableValueHolder#resetValue} method.
     * </p>

Boils down to the same but not anymore ambiguous.

[tandraschko]

in which versions we should fix it?

[BalusC]

Clarifying spec can only be done in next version (which is currently 5.0). But fixing impl can be done in any version.

[tandraschko]

AFAIR you cancelled already the released of 2.3.x in Mojarra right?

i just wonder what would be the best way for PF.