Repository navigation
Add the settings Forms class: controls, one saver and object-form checks - #29
Merged
Merged
Conversation
The settings package had no autoload key and no src/ directory, so the Forms class it is about to receive could not load, and phpstan.wordpress.neon covered core alone. Settings maps DeepWebSolutions\Framework\Settings\ to src/ through PSR-4, with no files entry, and src/ carries the index.php stub. phpstan.wordpress.neon analyzes packages/settings beside packages/core, with the same WordPress stubs and extensions. The lock records the new autoload map. Nothing was outdated, so this branch has no package update: composer outdated --direct listed nothing, and composer update and npm update left both locks unchanged. Assisted-by: Claude Code:claude-opus-5-5
The settings package has no way to render a field array's control or to tell whether WooCommerce would render that field disabled, which the options page, the WooCommerce bridge and the product panel all need. IsDisabledTest pins the disabled definition: a disabled key in custom_attributes, whatever its value or letter case, or a checkbox's truthy top-level disabled; never a radio's choice list, and never a top-level disabled on another type. RenderControlTest pins render_control() through DOM queries: escaping, the id and name, help linked by aria-describedby, a fieldset with a screen-reader legend for radio, checkboxes and a described checkbox, positional choice ids, the hidden presence inputs and their absence on disabled controls, radio's disabled choices by native key, custom attributes that never override the framework's own, placeholder only on text-like controls, core's sizing classes, step="any" on numbers, and render callables. All 59 fail, because Forms does not exist yet. Assisted-by: Claude Code:claude-opus-5-5
Settings could not render a field array's control, and the hosts that skip disabled fields had no shared definition of disabled to agree on. Forms is a final class with public static entry points, protected static helpers and a protected constructor. It declares the Field shape the hosts import. is_disabled() follows WooCommerce: a disabled key in custom_attributes, whatever its value or letter case, or a checkbox's truthy top-level disabled. render_control() returns the escaped control without its row label: a fieldset with a screen-reader legend for radio, checkboxes and a described checkbox; help in p.description paragraphs linked by aria-describedby; positional choice ids; a hidden no before an enabled checkbox and a hidden empty value before an enabled multi-value control; radio's disabled choices matched by native key; custom attributes that never override the framework's own and never name an event handler; placeholder only on text-like controls; core's sizing classes; and step="any" on numbers, because the default step makes a stored decimal block the form. The Field alias is a plain array shape. PHPStan 2.3.1 drops an optional key from an unsealed shape after ?? or isset(), so the narrowed field could not be passed on, while a plain shape still accepts extra keys at call sites, which keeps consumers free to add keys. Assisted-by: Claude Code:claude-opus-5-5
No saver could sanitize a field's submission, so the options page, the product panel and object forms would each need their own sanitizers. SanitizeValueTest pins sanitize_value(): each type's sanitizer, with email and url rejected rather than emptied when the sanitizer would empty or rewrite them; numbers in HTML's floating-point grammar, trimmed and finite; arrays, objects, booleans and null rejected by single-value types and ints and floats stringified; checkbox values mapped to yes and no; membership with integer-keyed options matched by their post strings and stored as strings; radio's disabled choices rejected; multi-value lists de-duplicated and re-indexed, with an empty string read as an empty list; validate called with the raw submission alone and its return stored as is; and empty-message WP_Error codes. SanitizeFieldsTest pins sanitize_fields(): entries only for defined, present and enabled fields, values and errors returned apart, a present null sanitized, and the values merging onto the stored row. All 74 fail, because neither method exists yet. Assisted-by: Claude Code:claude-opus-5-5
Hosts had nothing to turn a submission into stored values, and the WordPress sanitizers alone would erase a stored value whenever they emptied a typo: sanitize_email( 'ops@localhost' ) and esc_url_raw( 'htps://example.com' ) both return an empty string. sanitize_value() hands the unslashed submission alone to validate when the field has one and stores its return as is. Otherwise each type sanitizes it: text and textarea through core's text sanitizers, email and url only when the sanitizer would neither empty nor rewrite them, numbers in HTML's floating-point grammar, since a number input renders +1 or 1. as empty, and checkboxes as yes or no. Choices must be enabled choices of the field, compared as the strings a form posts, and multi-value lists come back de-duplicated and re-indexed. Single-value types reject arrays, objects, booleans, null and non-finite floats instead of casting them. Rejections are WP_Errors with a code and an empty message, for the host to word. sanitize_fields() returns the accepted values and the rejections apart, keyed by field id, for defined, present and enabled fields only, so callers merge the values onto the stored row with array_replace(). Radio's disabled-choice check moves into one helper that rendering and sanitizing share. Assisted-by: Claude Code:claude-opus-5-5
Object forms (post and user meta boxes) had no request check, and a check that verified only the form's nonce action would accept a nonce from the same form for any object, which lets a classic-editor Preview, whose autosave fires save_post for the revision with the whole form posted, write unsaved values onto the live post. VerifyObjectFormTest pins verify_object_form(): a nonce minted for the action joined to the object id, together with the object and plugin capabilities, passes; another author's post, another user's profile, a user editing themself without the plugin capability, a nonce for another action, for another object or for the bare action, and a missing or non-string nonce are refused without dying. All 9 fail, because the method does not exist yet. Assisted-by: Claude Code:claude-opus-5-5
Object forms had no request check, and binding the nonce to the form
alone would let a save_post fired for another object in the same
request pass it: a classic-editor Preview saves an autosave revision
with the whole form posted, and update_post_meta() redirects the
revision to its parent, so unsaved values would land on the live post.
verify_object_form() reads the nonce from the field named by the
action behind an is_string() guard, never dies, and verifies it against
the action joined to the object id, as WordPress core binds
update-post_{id} and update-user_{id}. It then requires the object
capability on that id and the plugin capability, which matters because
every user holds edit_user on themself.
Assisted-by: Claude Code:claude-opus-5-5
AGENTS.md mapped the settings family as an unnamed final class without sanitize_fields() or is_disabled(), said phpstan.wordpress.neon covered core alone, and its object-form convention named bare functions and no nonce binding or shared saver. The layout names Forms and its five entry points, the PHPStan line covers settings, convention 9 has object forms print their nonce bound to the object, call Forms::verify_object_form() and save through Forms::sanitize_fields(), and limitation 7 names Forms::render_control(). Assisted-by: Claude Code:claude-opus-5-5
render_attributes() dropped the framework's false and null attributes before adding the custom ones, so a custom attribute could set what the framework had decided to leave off: a checkbox stored as no with custom_attributes checked => false rendered checked="", which checks the box and posts yes on the next save. The framework's attributes and the custom ones are merged first, with the framework's keys winning, and only then are false and null values dropped, so every attribute the framework renders or leaves off stays its own. A regression test covers checked on a checkbox and multiple on a single select. Assisted-by: Claude Code:claude-opus-5-5
Three stored values changed on an untouched save or warned on render. esc_attr() leaves existing entities as they are, so a text value of Tom & Jerry rendered as value="Tom & Jerry", posted back as Tom & Jerry, and a choice keyed a&b posted a&b and failed its membership check. HTML parsing drops a newline right after a textarea's opening tag, so a value a validate callable had stored with a leading newline lost it. And a NAN value or custom attribute was cast to a string, which PHP 8.5 warns about. Attribute values have their ampersands encoded before esc_attr(), a textarea prints a newline after its opening tag for the parser to drop, and every rendered value goes through to_string(), which accepts only strings, ints, finite floats and bools. Each case has a regression test. Assisted-by: Claude Code:claude-opus-5-5
Label for attributes and help paragraph ids went through esc_attr() directly, while every other attribute went through render_attributes(), which encodes ampersands first. An id holding an entity-like sequence therefore reached the DOM in two spellings, so its labels and aria-describedby no longer pointed at their controls and help. The labels and help paragraphs build their attributes through render_attributes() too, leaving one escaping path for every attribute the class prints. A regression test covers a radio group and a described checkbox whose id holds an entity. Assisted-by: Claude Code:claude-opus-5-5
ahegyes
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The settings package had no code: nothing rendered a field array's control, sanitized its submission or verified an object form, so every later host (OptionsPage, the WooCommerce bridge, ProductDataTab, object forms) would have needed its own.
DeepWebSolutions\Framework\Settings\Formsis a final class with public static entry points, protected static helpers and a protected constructor. It declares theFieldarray shape the hosts import.render_control()returns the escaped control without its row label.descrender in a fieldset named by a screen-reader legend; help isp.descriptionlinked byaria-describedby; choices get positional ids.step="any".is_disabled()follows WooCommerce: adisabledcustom attribute, whatever its value, or a checkbox's truthy top-leveldisabled.sanitize_value()hands the unslashed submission alone tovalidatewhen the field has one. Otherwise each type sanitizes it:sanitize_email()would empty or rewrite it, and url whenesc_url_raw()would empty it, instead of erasing the stored value;Rejections are
WP_Errors with a code and an empty message for the host to word.sanitize_fields()returns accepted values and rejections apart, for defined, present and enabled fields, so hosts merge witharray_replace( $stored, $result['values'] ).verify_object_form()verifies a nonce bound to the object id, as core bindsupdate-post_{id}, plus the object and plugin capabilities, and never dies. Binding matters because a classic-editor Preview firessave_postfor an autosave revision with the whole form posted, andupdate_post_meta()redirects the revision to its parent.The settings package autoloads through PSR-4 only, PHPStan's WordPress profile covers it, and
PackageManifestsTestkeeps proving it needs only shared. AGENTS.md names the class and drops the cap on framework strings.Affected packages
wp-framework-bootstrapwp-framework-sharedwp-framework-corewp-framework-settingswp-framework-woocommerceChecklist
tests/Integration/Settings/Forms/holds 147 rows acrossRenderControlTest,SanitizeValueTest,SanitizeFieldsTest,VerifyObjectFormTestandIsDisabledTest, with rendered markup asserted throughDom\HTMLDocument.composer quality-checkpasses locally (Unit 200, Integration 641), pluscomposer test:integration:multisite(26) and PHP 7.4php -loverpackages/bootstrapREADME.md(the package READMEs stay pointer-form; AGENTS.md carries the file map)Formsis new)