Improvement for handling checkboxes in web forms (patch included) [SPR-2733]

[spring-projects-issues]

Dmitri Toubelis opened SPR-2733 and commented

The current implementation of dealing with checkboxes in web forms works well for simple backed objects like Type Boolean or String. In my case backend object is List of beans. And beans in that list needs to be updated rather then re-created. I also wrote a property editor for this object but it is only called if checkbox is checked, otherwise, the the backing object reset to null (I'm using this techchnic with hidden input fields). This is an example:

<c:forEach items="${status.value}" var="item" varStatus="stat">

<input type="checkbox" name="<c:out value="${status.expression}[${stat.index}]"/>" value="true" <c:if test="${item.selected}">checked="checked"</c:if>/><c:out value="${item.localizedName}" /><br/>

<input type="hidden" name="_<c:out value="${status.expression}[${stat.index}]"/>" value="false" />

</c:forEach>

The problem lays with WebDataBinder.checkFieldMarkers(...) method - it re-sets fields that are not present in a form by making call to WebDataBinder.getEmptyValue(...) and the latter just sets missing value to null for anything but boolean.

I would suggest a better approach and this is a patch for WebDataBinder.java:

--- WebDataBinder_orig.java 2006-05-15 10:42:14.000000000 -0400
+++ WebDataBinder.java 2006-10-19 14:46:50.000000000 -0400
@@ -165,9 +165,8 @@
PropertyValue pv = pvArray[i];
if (pv.getName().startsWith(fieldMarkerPrefix)) {
String field = pv.getName().substring(fieldMarkerPrefix.length());

---

if (getPropertyAccessor().isWritableProperty(field) && !mpvs.contains(field)) {

  Class fieldType = getPropertyAccessor().getPropertyType(field);

  mpvs.addPropertyValue(field, getEmptyValue(field, fieldType));

  if (!mpvs.contains(field)) {

                          mpvs.addPropertyValue(field, pv.getValue());
                  }
          }
  }

In contrast to original one, this approach does not create empty object but rather lets property edirors deal with it. And I think this is right thing to do - binded should not be dealing with data type conversions at all. Certainly, with this approach the hidden input field should contain "off" value of the field as versus of "just any" value from before.

I realise that this fix might interfere with backward compatibility and the new tag library, but I'm pretty sure it cures more problems than it creats new ones at the end of the day :-) So, I hope you consider it for the nearest upcoming release of your outstanding framework ;-)

---

Affects: 2.0 final

Attachments:

• WebDataBinder.patch (653 bytes)

Referenced from: commits 61b5428

2 votes, 3 watchers

[spring-projects-issues]

Juergen Hoeller commented

This is an interesting idea: essentially not specifying a field marker parameter, but rather a field reset parameter. We can certainly consider adding this, but probably not within the present field marker prefix mechanism, which needs to remain backwards-compatible. Preferably, we could add a separate "fieldResetPrefix" (maybe "!" by default) that expects parameters with a specific off value instead of just an arbitrary marker value.

Juergen

[spring-projects-issues]

Scott Andrews commented

Introduced default field prefix of '!', which can be overridden with WebDataBinder#setFieldDefaultPrefix. If a field is otherwise not present, the default value is used for the field. Field markers for the same field are ignored.

The behavior of field markers is unchanged.

Thanks for the patch.