processwire-issues icon indicating copy to clipboard operation
processwire-issues copied to clipboard

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

Open kixe opened this issue 1 year ago • 3 comments

Short description of the issue

InputfieldFile::getItemInputfields() does not check against the value 0 (not null) 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' (string)) in DB 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 explicit 0, because it can be checked by default, which happens if the value == null

Actual behavior

Currently the function InputfieldFile::getItemInputfields() adds the attribute 'checked' ALWAYS if "checked by default" is selected, because it does not make any difference between the 3 types of zero values (0, '0' or null). But it should add "checked" only if it is exactly null.

Optional: Suggestion for a possible fix

Would be great to add the following line

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

https://github.com/processwire/processwire/blob/44e6c89e35e539554b59b374fd7a27212d184eac/wire/modules/Inputfield/InputfieldFile/InputfieldFile.module#L1587

Setup/Environment

  • ProcessWire version: 3.0.239
  • PHP version: 8.2
  • 3rd party module: https://github.com/kixe/FieldtypeCheckboxExtended

kixe avatar Jun 17 '24 06:06 kixe

@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?

ryancramerdesign avatar Jun 20 '24 15:06 ryancramerdesign

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.

kixe avatar Jun 21 '24 02:06 kixe

@ryancramerdesign Any chance of seeing some progress here? Even if this is not related to the core FieldtypeCheckbox, which does not support ‘checked by default’ and therefore makes no difference between "0" (string), 0 (int) and NULL, it could be an improvement for the future.

kixe avatar May 28 '25 18:05 kixe