FIX: checkbox custom options with zero or one predefined option

[ObitoTM]

https://github.com/Crocoblock/issues-tracker/issues/19809

[github-actions]

🤖 AI PR Review

Risk level: medium

Review

Summary

• This PR fixes handling of custom checkbox options when a field has zero or one predefined option. Changes touch frontend JS (src + built bundle) and the PHP block render for checkbox option fields.

What I reviewed

• modules/option-field/assets/src/frontend/blocks/checkbox/CheckboxData.js
• modules/option-field/assets/src/frontend/blocks/checkbox/functions/sanitizeCheckbox.js
• modules/option-field/blocks/checkbox/block-render.php
• built assets (checkbox.js and checkbox.asset.php)

Positive

• The change prevents the single empty custom checkbox input from being removed (now it's cleared and disabled) which fixes the UX/submit edge-case described in the issue.
• sanitizeCheckbox now uses the central removeCustomOption logic instead of duplicating DOM removal behavior — good consolidation.
• The JS changes make sanitizeCheckbox return a value in all paths (previous early-return returned undefined) — that was likely a bug and is now fixed.
• get_name_suffix now accounts for allowed custom options and will produce a proper [] suffix when needed — this aligns server-side naming with array semantics for custom options.

Potential issues, security and compatibility concerns

1. Missing null-safety for first node in setNode

   • File: modules/option-field/assets/src/frontend/blocks/checkbox/CheckboxData.js
   • Code: this.rawName = firstNode.name || getCustomCheckboxInput( firstNode ).name;
   • Risk: if this.nodes[0] is undefined for some reason (unexpected DOM structure), firstNode could be undefined and getCustomCheckboxInput will receive undefined. Add a safe check (if firstNode) or fallback to '' to avoid runtime errors.

2. get_attributes_string_save duplication / attribute escaping

   • File: modules/option-field/blocks/checkbox/block-render.php
   • Change: Builder_Helper::attrs(...) is used for some attributes, and then $this->get_attributes_string_save() is appended into the input element. Previously the class attr was composed directly inside Builder_Helper::attrs; in this patch class attr was removed from attrs and likely expected to be present in get_attributes_string_save(). Verify get_attributes_string_save() provides the necessary attributes (class, data-*, aria, name, id, etc.) and that attributes aren’t duplicated (same attribute emitted twice). Also verify get_attributes_string_save() properly escapes attribute values (esc_attr) — exposing unescaped attributes could be an XSS vector.

3. get_name_suffix change is breaking-seeming for some edge cases

   • File: modules/option-field/blocks/checkbox/block-render.php
   • Change: name suffix now includes [] if custom_option.allow is set. This is intentional, but it changes the posted field name for forms that previously had exactly one predefined option and custom options allowed. That may change how existing integrations (server-side mappings, actions that expect a scalar) receive values. This is a functional change that may affect add-ons/snippets. Please confirm this is intended and document in release notes.

4. Removal behavior and the single-checkbox edge-case

   • Files: CheckboxData.js and sanitizeCheckbox.js
   • The removeCustomOption keeps one empty, disabled input when only one node exists. This is good UX; please ensure this behavior is covered by tests (see tests below). Also ensure that disabled inputs don't get posted by browsers (disabled inputs are not posted) so clearing + disabling is the correct way to represent “no custom value” on submission.

5. Minified bundle updated but ensure build pipeline parity

   • Files: modules/option-field/assets/build/checkbox.js and checkbox.asset.php
   • The committed built/minified file changed. Ensure the build was done using the project’s standard bundler so source <-> built parity is kept and that sourcemaps (if expected in repository) are updated as necessary.

Recommendations / small fixes

• Add a defensive check before using firstNode in setNode (return early or fallback to a safe name). Example: if (!firstNode) { this.rawName = ''; } else { ... }
• Verify getCustomCheckboxInput is imported/available in file scope where used (it is used in other functions — ensure no accidental rename/import mismatch).
• In block-render.php, double-check that Builder_Helper::attrs(...) + get_attributes_string_save() produce consistent and non-duplicated attributes. If get_attributes_string_save() already contains data-custom/data-field-name, remove duplicates from Builder_Helper::attrs or vice versa.
• Add unit / integration tests covering the following scenarios:
  • Checkbox field with zero predefined options + custom option allowed: ensure HTML renders a disabled empty input and that submissions produce an empty value (no nulls) as intended.
  • Checkbox field with one predefined option + custom option allowed: ensure name suffix behavior and submission shape (scalar vs array) remain consistent with expected transform pipeline.
  • Large-form behavior (several custom options added/removed) to ensure no memory leak or watcher misbehavior.
• Add a brief inline comment in CheckboxData.removeCustomOption documenting why we disable+clear the last input instead of removing it (helps future maintainers).

Tests missing

• I don't see automated tests or e2e tests for the single/no-option custom checkbox flow. This is a high-risk area for data mapping and should have a test.

Summary judgment

• This PR fixes the reported bug and consolidates DOM removal logic, but it touches how names/inputs are generated and could affect integrations. Add the small null-check, verify attribute escaping/duplication in PHP markup, and add tests for the edge cases.

Files of interest

• modules/option-field/assets/src/frontend/blocks/checkbox/CheckboxData.js
• modules/option-field/assets/src/frontend/blocks/checkbox/functions/sanitizeCheckbox.js
• modules/option-field/blocks/checkbox/block-render.php

Suggested changelog entry

- FIX: checkbox custom options with zero or one predefined option (fix custom option removal and name handling)