Fix/textarea array coercion#1279
Merged
Merged
Conversation
Two unrelated nits flagged by PHPCS but never addressed: - Line 301: align equals sign with surrounding assignment (auto-fixable Generic.Formatting.MultipleStatementAlignment warning). - Line 466: remove stray blank line before block comment in validate_number_field (Squiz.Commenting.BlockComment.HasEmptyLineBefore error). Lint-only cleanup; no behaviour change.
Setup Wizard / Settings save fataled with:
Uncaught TypeError: addslashes(): Argument #1 ($string) must be of
type string, array given in inc/ui/class-field.php:504
This happened when a textarea or wp_editor field reached
Field::validate_textarea_field() with a non-string value. Two real
sources observed:
1. Settings::save_settings() iterates ALL registered sections, not just
the fields posted in the current Setup Wizard step. For fields not
in $settings_to_save it falls back to the saved DB value as
$existing_value. If a previous corruption stored an array / object /
Closure (see #1148), that value flows straight back into
set_value() -> sanitize() -> validate_textarea_field() and fatals
under addslashes()/stripslashes() on PHP 8+.
2. A form posting name="field[]" syntax (intentional or otherwise)
delivers an array in $_POST.
Fix: harden validate_textarea_field() with a coerce_textarea_value()
helper that:
- returns the value unchanged when already a string,
- returns '' for null / array / object (preventing the TypeError and
preserving the historical 'drop malformed values' behaviour rather
than propagating corruption), and
- casts other scalars (int / float / bool) to their string form.
Add 6 regression tests covering string round-trip, array, object,
null, and scalar coercion for both textarea and wp_editor types.
Verification:
- vendor/bin/phpunit --filter 'WP_Ultimo\\UI\\Field_Test' -> 25/25 pass
- vendor/bin/phpstan analyse on both files -> clean
- vendor/bin/phpcs on both files -> only a pre-existing json_encode
warning in test code, none in modified ranges
Contributor
📝 WalkthroughWalkthroughThis PR adds defensive input coercion to textarea and wp_editor field sanitization to prevent TypeErrors on non-string values, backed by six regression tests, and simultaneously restructures TODO.md and PLANS.md into standardized TOON-compatible template formats with defined metadata sections and empty placeholders for tooling integration. ChangesTextarea field input coercion
Documentation template restructuring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
todo/PLANS.md (1)
42-147:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftInconsistency between TOON metadata and actual plan content.
The TOON directive at lines 46-47 declares
active_plans[0](zero active plans), but lines 49-147 contain a substantial active PayPal integration plan with execution phases, context, and file references. This inconsistency will cause TOON parsing tools to misreport the plan count and break automated analytics.Additionally, the preserved plan content doesn't follow the standardized template structure defined at lines 164-214 (which includes sections for Purpose, Development Environment, Linkage, Progress checkboxes, Decision Log, and Surprises & Discoveries).
Consider one of the following approaches:
- Reformat the PayPal plan to match the new template structure, and update the TOON metadata count to
[1]- Move the plan content to a separate file (e.g.,
todo/tasks/p001-paypal-review.md) and link it from a minimal Active Plans entry- Complete the migration in a follow-up if this restructuring is intentionally incremental
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@todo/PLANS.md` around lines 42 - 147, The TOON metadata declares active_plans[0] but the file actually contains a full PayPal plan (references like t523a–t523e, Phase 1/2/3 and the Plan Template), causing a mismatch; either (A) update the TOON directive to active_plans[1] and refactor the PayPal content to conform exactly to the Plan Template sections (Purpose, Development Environment, Linkage, Progress checkboxes, Decision Log, Surprises & Discoveries), or (B) move the detailed PayPal plan out to a new file (e.g., todo/tasks/p001-paypal-review.md), replace the Active Plans entry with a minimal TOON pointer/summary, and ensure the TOON metadata and plan body are consistent (adjust TOON index and any tags) so parsers reading the TOON directive (active_plans[0]) match the actual plan count and structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@todo/PLANS.md`:
- Around line 42-147: The TOON metadata declares active_plans[0] but the file
actually contains a full PayPal plan (references like t523a–t523e, Phase 1/2/3
and the Plan Template), causing a mismatch; either (A) update the TOON directive
to active_plans[1] and refactor the PayPal content to conform exactly to the
Plan Template sections (Purpose, Development Environment, Linkage, Progress
checkboxes, Decision Log, Surprises & Discoveries), or (B) move the detailed
PayPal plan out to a new file (e.g., todo/tasks/p001-paypal-review.md), replace
the Active Plans entry with a minimal TOON pointer/summary, and ensure the TOON
metadata and plan body are consistent (adjust TOON index and any tags) so
parsers reading the TOON directive (active_plans[0]) match the actual plan count
and structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: edc8c2c5-ab24-41ec-af72-ff8d7f828d43
📒 Files selected for processing (4)
TODO.mdinc/ui/class-field.phptests/WP_Ultimo/UI/Field_Test.phptodo/PLANS.md
|
Performance Test Results Performance test results for e8f2640 are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
Collaborator
Author
|
Completed via PR #1279, merged to main. Merged by deterministic merge pass (pulse-wrapper.sh). Neither MERGE_SUMMARY comment nor PR body text was available. aidevops.sh v3.19.5 spent 2m on this as a headless bash routine. |
superdav42
added
the
review-feedback-scanned
superdav42
added a commit
that referenced
this pull request
May 27, 2026
…nsive coerce from #1279 (#1295) The Setup Wizard fatal that #1279 worked around had a single-character root cause, not a Field-layer defect: Credits::get_default_custom_credit_html was declared `protected`, but inc/class-credits.php:103 registers the field default as the array callable `[$this, 'get_default_custom_credit_html']`. `WP_Ultimo\Settings::save_settings()` resolves field defaults with `is_callable($field_default)` from a different class scope. PHP returns `false` for `is_callable([$instance, 'protected_method'])` evaluated outside the declaring class. With the callable unresolvable, the literal array `[$instance, 'get_default_custom_credit_html']` survived as the field value, landed in `Field::validate_textarea_field()`, hit `addslashes()`, and raised: Uncaught TypeError: addslashes(): Argument #1 ($string) must be of type string, array given in inc/ui/class-field.php:504 The same broken JSON also poisoned the wizard data-state JSON used to initialise the Vue form, which is why the form rendered with no settings fields on a fresh install. Both symptoms — empty form and the save fatal — disappear once the method is public. Changes: - inc/class-credits.php: visibility flipped from `protected` to `public`, with a docblock explaining the field-default callable contract so this bug class cannot silently regress. - inc/ui/class-field.php: reverts the defensive `coerce_textarea_value()` added by #1279. The function was a downstream band-aid; with the source fixed, no non-string value can reach this validator from this code path. Preserves the two unrelated PHPCS lint cleanups from #1279's first commit (alignment at line 301 and blank line before block comment at line 466). - tests/WP_Ultimo/Credits_Test.php: adds two regression tests: * `test_get_default_custom_credit_html_is_callable_externally` asserts both `is_callable([$instance, 'method'])` and ReflectionMethod::isPublic so a future visibility downgrade fails CI. * `test_get_default_custom_credit_html_returns_non_empty_string` asserts the resolved default value is a string suitable for textarea validation. - tests/WP_Ultimo/UI/Field_Test.php: reverts the six textarea-coerce tests added by #1279; they covered the removed band-aid. Audit: surveyed every `'default' => [$this, '...']` and `'options' => [$this, '...']` callable across inc/. `Credits::get_default_custom_credit_html` was the only target with restricted visibility. The two other registered field defaults (`Domain_Manager::default_domain_mapping_instructions`, `Settings::get_default_company_country`) and all seven options callables are already public — no further code changes needed. Verification: vendor/bin/phpunit --filter 'Credits_Test' # 17/17 pass vendor/bin/phpcs inc/class-credits.php inc/ui/class-field.php \ tests/WP_Ultimo/Credits_Test.php # 0 errors vendor/bin/phpstan analyse inc/class-credits.php \ inc/ui/class-field.php tests/WP_Ultimo/Credits_Test.php # No errors Live-install repro on a fresh install before/after the change: - Before: wizard step 2 renders with no fields; submitting fatals at class-field.php:504 with the addslashes TypeError above. - After: all 11 fields render and save without error.
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
1 participant
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 by CodeRabbit
Bug Fixes
Documentation
Tests