Skip to content

Tabbed YAML settings: saving one tab resets booleans on the others to false #64

Description

@titus-toia

Summary

On a tabbed settings page built from YAML, saving one tab writes false to every boolean on every
other tab
, and forces every repeater to '[]'. The user never sees the fields being reset, and
there is no error.

Verified against tangible/object v1.0.1.

Cause

Two behaviours that are each reasonable alone:

  1. src/Settings/SettingsRenderer.php (~line 123) renders only the active tab's fields, so the
    POST body contains only that tab's inputs:
// Render active tab content
foreach ($tabs as $tab) {
    $tabKey = sanitize_key($tab['label']);
    if ($currentTab !== $tabKey) {
        continue;
    }
  1. src/DataView/RequestRouter.php::extract_post_data() (~line 446) iterates every field in the
    schema
    , not the fields that were rendered:
foreach ( $this->config->field_configs as $name => $config ) {
    $has_value = isset( $nested[ $name ] ) || isset( $params[ $name ] );

    if ( ! $has_value ) {
        if ( $this->registry->get_dataset_type( $type ) === DataSet::TYPE_BOOLEAN ) {
            $data[ $name ] = false;          // ← unchecked-checkbox handling
        }
        if ( $type === 'repeater' ) {
            $data[ $name ] = '[]';
        }
        continue;
    }
    ...
}

A checkbox on an inactive tab is absent from POST, which is indistinguishable from a box the user
deliberately unticked — so it is written false.

text fields are unaffected: they hit the bare continue, never enter $data, and
RequestHandler/SingularHandler.php::update() iterates $data only.

Steps to reproduce

  1. Define YAML settings with two tabs, each containing at least one boolean field, both defaulting
    to true.
  2. Save tab A.
  3. Read the option back — tab B's boolean is now false.

Impact

Any safety-relevant checkbox becomes unsafe to put on a multi-tab settings page: an unrelated save
switches it off silently, and it is discovered much later, if ever. In one plugin this affected two
guards whose whole job was preventing a destructive operation, and the workaround was to demote both
to constants — losing the configurability the settings page existed to provide.

Suggested fix

Scope the missing-boolean/repeater defaults to fields that were actually rendered. RequestRouter
already has the layout, so it can restrict the loop to the submitted tab's field set. Alternatively
SettingsRenderer could emit hidden inputs carrying the current values for inactive tabs — cheaper,
but it makes the form carry state it does not display.

Happy to send a PR if the first approach is the preferred shape.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions