Skip to content

fix(schema): accept readonly and disabled where ACF renders them - #38

Merged
parisek merged 2 commits into
mainfrom
fix/readonly-disabled-on-input-fields
Sep 4, 2026
Merged

fix(schema): accept readonly and disabled where ACF renders them#38
parisek merged 2 commits into
mainfrom
fix/readonly-disabled-on-input-fields

Conversation

@parisek

@parisek parisek commented Sep 4, 2026

Copy link
Copy Markdown
Owner

From-Project

Found while running composer lint:acf-json on fellows, whose flat field group marks two import-owned fields (better_id, better_roomtype_id) read-only so editors cannot desync them from the CSV import that owns them:

✗ templates/flat/acf.json (acf)
    /fields/2 — Unevaluated object properties not allowed: readonly
    /fields/3 — Unevaluated object properties not allowed: readonly

Why

ACF Pro supports readonly and disabled on ten field types and passes both into the rendered input. No schema declared either key, and field-item.schema.json sets unevaluatedProperties: false — so a field group using ACF's own read-only inputs fails validation. Per the consuming doctrine there is no local ignore and no per-project override, so the project's only lint-clean option was to delete configuration that works.

Verified against ACF Pro 6.x source rather than the documentation, which lists neither setting:

Type Where
text class-acf-field-text.php:71
textarea class-acf-field-textarea.php:59
number class-acf-field-number.php:61
range class-acf-field-range.php:60
email class-acf-field-email.php:58
url class-acf-field-url.php:52
select class-acf-field-select.php:283-287
date_picker class-acf-field-date-picker.php:128
date_time_picker class-acf-field-date-time-picker.php:141
time_picker class-acf-field-time-picker.php:78

What

Adds to those ten per-type refs, in both src/templates/refs/ and schemas/refs/ per the source-of-truth rule in AGENTS.md:

"readonly": { "enum": [0, 1], "description": "ACF renders this into the input's readonly attribute." },
"disabled": { "enum": [0, 1], "description": "ACF renders this into the input's disabled attribute." }

Per type, deliberately not in refs/field.schema.json. The base schema would accept both keys on image, repeater, true_false and everything else, where ACF silently ignores them — the dead configuration this package already reports elsewhere (the WPML repeater message calls it that by name). enum: [0, 1] matches required and allow_in_bindings in the base schema; ACF truthy-checks the value and serialises 0/1.

No new field type, so FIELD_TYPE_ORDER and the generated root schemas are untouched.

Tests

Two fixtures, one per half of the decision:

  • valid/starter_theme/readonly-inputsreadonly/disabled on text, number, select, date_picker.
  • invalid/readonly-on-imagereadonly on an image. This is the load-bearing one: without it, a later move of these keys into the base schema would pass the whole suite.

Checked by negative control that the invalid fixture fails only on this key — removing readonly from that same document makes it validate:

s readonly:   nevalidni   → {"/fields/0": "Unevaluated object properties not allowed: readonly"}
bez readonly: VALID

composer check green locally: PHPUnit 205 passed / 1 skipped, PHPStan level 8 clean. The real fellows flat/acf.json validates against the patched schemas.

Not proposed

Loosening unevaluatedProperties on field-item.schema.json. It is what makes a typo'd key an error, and trading that for this would be a bad exchange.

Draft — opening for review rather than merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_012GJnbedTAFFLZX5Z8MEnTB

ACF Pro passes `readonly` and `disabled` into the rendered input for ten
field types, but no schema declared either key and `field-item.schema.json`
sets `unevaluatedProperties: false`. A field group using ACF's own
read-only inputs therefore failed validation, and the package has no local
ignore to fall back on -- so the consuming project had no lint-clean option
short of removing configuration that works.

Verified against ACF Pro 6.x rather than the documentation, which does not
list either setting:

  text              class-acf-field-text.php:71
  textarea          class-acf-field-textarea.php:59
  number            class-acf-field-number.php:61
  range             class-acf-field-range.php:60
  email             class-acf-field-email.php:58
  url               class-acf-field-url.php:52
  select            class-acf-field-select.php:283-287
  date_picker       class-acf-field-date-picker.php:128
  date_time_picker  class-acf-field-date-time-picker.php:141
  time_picker       class-acf-field-time-picker.php:78

Added to those ten refs only, NOT to `refs/field.schema.json`. The base
schema would accept the keys on `image`, `repeater`, `true_false` and every
other type, where ACF silently ignores them -- exactly the dead
configuration this package reports elsewhere (the WPML repeater message
says so in as many words). `enum: [0, 1]` matches `required` and
`allow_in_bindings` in the base schema; ACF truthy-checks the value and
serialises 0/1.

Both copies edited, template and distribution, per the source-of-truth rule
in AGENTS.md. No new field type, so `FIELD_TYPE_ORDER` and the generated
root schemas are untouched.

Fixtures cover both halves of the decision: a valid group carrying
`readonly`/`disabled` on text, number, select and date_picker, and an
invalid one putting `readonly` on an image. The second is the load-bearing
one -- without it a later move of these keys into the base schema would
pass every test. Checked by negative control that the invalid fixture fails
ONLY on this key: removing `readonly` from it makes the same document
validate.

Found while linting a real project (fellows), whose `flat` field group
marks two import-owned fields read-only; that file validates against the
patched schemas.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GJnbedTAFFLZX5Z8MEnTB (petr@pari.cz)
@parisek parisek self-assigned this Sep 4, 2026
Two independent reviews of the first commit. Both findings below were
verified against the ACF source before acting.

PASSWORD WAS MISSING. `class-acf-field-password.php:52` delegates its whole
render to the text field, which is where `readonly`/`disabled` are passed
into the input -- so password honours both and the schema still rejected
them. The first commit's evidence was gathered by grepping each field class
for the literal `'readonly'`, which cannot see a type that inherits the
behaviour. Checked the whole directory for that pattern afterwards:
`password` is the only delegating type, so the blind spot cost exactly one.

Control run rather than assumed: a password field with `readonly: 1`
validates against the patched schemas and fails against the previous
commit's, with the error naming the key.

FIXTURE now covers all eleven accepted types instead of four. The previous
fixture would have stayed green with password missing, which is how the gap
survived the first round.

Invalid fixture renamed `readonly-on-image` -> `image-readonly-prop`, to
match the `<type>-<problem>` convention every sibling uses
(`image-return-format-url`, `file-unknown-prop`, `button_group-bad-layout`).

CHANGELOG now cites the ACF source behind the type list and names the
downstream project, as the 0.7.3 and 0.7.5 entries do, and warns that the
list is a claim about ACF internals that needs re-checking on upgrades.

CONSIDERED AND NOT CHANGED, both raised as findings:

- `readonly` on `select` is inert in the browser: ACF sets the attribute
  (`class-acf-field-select.php:283`) but HTML ignores `readonly` on
  `<select>`, and Select2 reads `disabled`. Kept, because the criterion here
  is what ACF writes into the input, not what the browser then does with it.
  Modelling browser semantics would be a different and much larger promise.
- `enum: [0, 1]` rejects `readonly: true`. Kept for consistency with
  `required` and `allow_in_bindings` in `refs/field.schema.json`, which use
  the same shape; acf.json is an export format and ACF's admin serialises
  0/1. Worth revisiting only if ACF is shown to export a boolean.
- `disabled` on `radio`/`checkbox` is a list of choice values, not a flag
  (`class-acf-field-checkbox.php:265,310`, `class-acf-field-radio.php:158`).
  Real, and deliberately out of scope: it needs its own array shape, and
  folding a second shape into this change would bundle two decisions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GJnbedTAFFLZX5Z8MEnTB (petr@pari.cz)
@parisek

parisek commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Agent review record — two independent reviewers on 4b6f41e, 2026-09-04, deliberately given non-overlapping lenses and different models: Codex (via codex-rescue) on correctness against the ACF Pro source, Claude Fable 5 on this repo's own doctrine, versioning and downstream impact. Neither could see the other's output. Every finding below was verified against the code before acting.

Fixed in ccf9c29

  • password was missing from the accepted types (Codex). class-acf-field-password.php:52 delegates its entire render to the text field, where both keys are passed into the input. The first commit's evidence came from grepping each field class for the literal 'readonly', which cannot see a type that inherits the behaviour. Swept the directory for that delegation pattern afterwards — password is the only one, so the blind spot cost exactly one type. Verified by control: a password field with readonly: 1 validates now and failed on 4b6f41e, with the error naming the key.
  • Fixture covered only 4 of the accepted types (Codex, and Fable independently on the weakness of min_violations: 1). Now covers all eleven. The old fixture would have stayed green with password missing — which is exactly how that gap survived round one.
  • Invalid fixture name broke the sibling convention (Fable). Every sibling is <type>-<problem> (image-return-format-url, file-unknown-prop, button_group-bad-layout); ours was <problem>-on-<type>. Renamed to image-readonly-prop.
  • CHANGELOG lacked the ACF source citation and the origin project (Fable) that the 0.7.3 and 0.7.5 entries carry. Added, plus a note that the type list is a claim about ACF internals needing re-checking on ACF upgrades.

Rejected, with reasons

  • readonly on select is inert in the browser (Codex). Correct as an observation: ACF sets the attribute (class-acf-field-select.php:283) but HTML ignores readonly on <select>, and Select2 reads disabled. Kept anyway — the criterion here is what ACF writes into the input, not what the browser does with it afterwards. Modelling browser semantics is a different and much larger promise than this schema makes.
  • enum: [0, 1] rejects readonly: true (Codex; Fable observed the same behaviour without treating it as a defect). Kept for consistency with required and allow_in_bindings in refs/field.schema.json, which use exactly this shape. acf.json is an export format and ACF's admin serialises 0/1. Worth revisiting if ACF is ever shown to export a boolean here.

Noted, deliberately out of scope

  • disabled on radio/checkbox is a list of choice values, not a flag (Codex) — verified at class-acf-field-checkbox.php:265,310 and class-acf-field-radio.php:158. Genuinely unsupported by the schema today, but it needs its own array shape; folding a second shape into this change would bundle two decisions into one PR. Worth a follow-up issue.
  • The valid fixture is synthetic but sits under a project name (Fable). tests/fixtures/valid/ reads as a per-project corpus of captured exports (real modified timestamps, ACF-shaped keys), and this one is hand-written. Not changed unilaterally: the clean fixes are either capturing it from a live install or adding a valid/synthetic/ sibling, and inventing a top-level convention in this repo is the maintainer's call, not a reviewer's. Flagging rather than deciding.

Where they converged

On the fixtures being too weak — reached from opposite directions, Codex from coverage of the type matrix and Fable from the looseness of min_violations: 1. Both also independently confirmed, by running the validator rather than reading the schema, that the allOf / if-then / unevaluatedProperties: false composition does what the PR claims: the keys are evaluated inside the per-type branch and rejected outside it. That was the part most likely to be subtly wrong in draft 2020-12, and it is the strongest evidence in this review precisely because neither reviewer took the other's word for it.

They disagreed on the merge call — Fable concluded nothing blocked merge, Codex found two High findings. That divergence is an artefact of the lenses I assigned, not of the code: Fable was explicitly told not to check the ACF facts, which is where both High findings lived. Worth recording so the split is not mistaken for one reviewer being wrong.

Reported by Claude Code (Claude Opus 5), 2026-09-04.

@parisek
parisek marked this pull request as ready for review September 4, 2026 14:07
@parisek
parisek merged commit 7f7db04 into main Sep 4, 2026
4 checks passed
@parisek
parisek deleted the fix/readonly-disabled-on-input-fields branch September 4, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant