search

processwire / processwire-issues

ProcessWire issue reports.
44 stars 2 forks source link

InputfieldFile::getItemInputfields() does not check against value 0 to uncheck a checkbox if checked is default. #1940

Open kixe opened 3 weeks ago

kixe commented 3 weeks ago

Short description of the issue

InputfieldFile::getItemInputfields() does not check against the value 0 to uncheck a checkbox if "checked" is the default value. This doesn't matter for the core type of FieldtypeCheckbox, as the "checked by default" option is still not available (which is a lack of default functionality). I use a custom FieldtypeCheckbox that always submit and stores the value (checked = 1, unchecked = 0) which allows to provide the "checked by default" option in field settings. https://github.com/kixe/FieldtypeCheckboxExtended

Expected behavior

If custom file / image field is of type Checkbox, the attribute 'checked' should be removed if value is empty, because it can be checked by default.

Actual behavior

Currently the function InputfieldFile::getItemInputfields() only adds the attribute if the value is not empty.

Optional: Suggestion for a possible fix

Would be great to add the following line

 // add after LINE 1572 if($value) $f->attr('checked', 'checked');
else if(!$value) $f->attr('checked', ''); // remove if previously assigned

Setup/Environment

ryancramerdesign commented 3 weeks ago

@kixe It's possible I don't comprehend the issue yet, and I'm not yet clear on how to observe/reproduce the issue? But what you describe sounds like the behavior it's supposed to use if the autocheck option is enabled. Does InputfieldFile need to use this option?

kixe commented 3 weeks ago

Does InputfieldFile need to use this option?

No, becauce 'autocheck' comes into play only for the ckeckedValue.

We should not confuse InputfieldCheckbox and FieldtypeCheckbox. InputfieldFile::getItemInputfields() is based on Fieldtypes assigned to a template and not directly on Inputfields. The code line 1571: else if ($f instanceof InputfieldCheckbox) { // ... is based on the assumption that it is the Inputfield related to the core type FieldtypeCheckbox, which makes no difference between a non-existent value (null) and the value "0" = not checked. On the other hand my FieldtypeCheckboxExtended using InputfieldCheckbox as well, makes this difference, a bit similar to FieldtypeInteger in combination with the option "0" != null.

If FieldtypeCheckboxExtended is used with custom fields and the provided option default=checked is selected, the 'ckecked' attribute in the Inputfield is already set when the Inputfield is called by InputfieldFile::getItemInputfields() and would have to be removed again here if the value in the DB (filedata column) is "0". However, this check does not take place.

I would like to see FieldtypeCheckboxExtended compatible with custom file fields. Currently there is no option to solve this via hook.