Fix element saving broken by unchecked-checkbox submission change#442
Conversation
The previous commit stopped submitting empty values for unchecked
checkboxes. This broke element saving in ViewMapAddModify: the toggle_<attr>
checkboxes control whether an attribute overrides its inherited value, and
the PHP code used has_var('toggle_' . $attr) to detect whether a toggle was
rendered in the form at all (vs. the attribute being a must-field with no
toggle). With unchecked toggles no longer submitted, has_var returned false
for unchecked toggles too, causing all toggled-off attributes to be treated
as "no toggle = include unconditionally", which selected all elements.
Fix by rendering a hidden sentinel field (_has_toggle_<attr>) alongside each
toggle checkbox. has_var now checks the sentinel, which is always submitted
as a hidden field regardless of the checkbox state, so the distinction
between "toggle present but unchecked" and "no toggle rendered" is preserved.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, well-targeted fix for the regression from #439, with a clear sentinel-field pattern and matching processing change.
Extended reasoning...
Overview
This PR fixes a regression introduced by #439, which stopped getFormParams() from submitting empty values for unchecked checkboxes. In ViewMapAddModify.php, the processing code relied on has_var('toggle_' . $attr) to distinguish a rendered-but-unchecked toggle (= use inherited) from a non-existent toggle (= must-attribute, always include). With unchecked toggles no longer submitted, that distinction collapsed, so toggled-off attributes were being saved unconditionally.
Security risks
None. This is a server-side form-processing fix that restores correct attribute filtering. No auth, crypto, or permission code is touched. The new _has_toggle_* hidden field is just a sentinel marker and is correctly excluded from the attribute loop by the existing str_starts_with($attr, '_') skip rule.
Level of scrutiny
Low. Two-file diff: a one-line changelog and ~7 lines of PHP (one render-side hidden() call plus a one-token rename in the processing condition). The fix mirrors a well-known web-form sentinel pattern, the rationale is documented in an inline comment, and the change is self-consistent — render adds the sentinel, processing checks the sentinel.
Other factors
The bug hunting system found no issues. The exclude logic in filterMapAttrs already skips any attribute whose name begins with _, so the new sentinels do not leak into $this->attrs. The get_checkbox('toggle_' . $attr) call still works as before for the actual checked/unchecked decision.
1 participant
Summary
Followup to #439.
getFormParams()from submitting empty values for unchecked checkboxes. This broke element saving: thetoggle_<attr>checkboxes inViewMapAddModifycontrol whether an attribute overrides its inherited value. The PHP processing code usedhas_var('toggle_' . $attr)to distinguish "toggle rendered but unchecked (= use inherited)" from "no toggle rendered at all (= must-attribute, always include)". With unchecked checkboxes no longer submitted,has_varreturned false for unchecked toggles too, so toggled-off attributes were incorrectly included unconditionally — causing all attributes to be saved as if selected.Changes
ViewMapAddModify.phprender: add a hidden sentinel field_has_toggle_<attr>alongside each toggle checkbox. Hidden fields are always submitted, so sentinel presence reliably signals "this toggle exists in the form."ViewMapAddModify.phpprocessing: switchhas_var('toggle_' . $attr)tohas_var('_has_toggle_' . $attr), restoring the correct distinction between unchecked toggle and absent toggle.