Fix config form value & Simplify CspConfigForm#5533
Open
TheSyscall wants to merge 5 commits into
Open
Conversation
TheSyscall
force-pushed
the
fix-config-form-value
branch
from
July 7, 2026 13:34
3735397 to
5daeece
Compare
jrauh01
requested changes
Jul 8, 2026
jrauh01
requested changes
Jul 10, 2026
jrauh01
left a comment
jrauh01
left a comment
Contributor
There was a problem hiding this comment.
I think it would make sense to add a test case to avoid the "default override" error in future changes.
TheSyscall
force-pushed
the
fix-config-form-value
branch
from
July 10, 2026 12:43
cf4993d to
21f87b6
Compare
jrauh01
requested changes
Jul 13, 2026
| }; | ||
| $form->ensureAssembled(); | ||
|
|
||
| $this->assertSame('defaultvalue', $form->getElement('mysection__key')->getValue()); |
Contributor
There was a problem hiding this comment.
Why not use this?
Suggested change
| $this->assertSame('defaultvalue', $form->getElement('mysection__key')->getValue()); | |
| $this->assertSame('defaultvalue', $form->getValue('mysection__key')); |
TheSyscall
force-pushed
the
fix-config-form-value
branch
from
July 13, 2026 06:23
21f87b6 to
932b316
Compare
TheSyscall
marked this pull request as draft
July 15, 2026 09:48
TheSyscall
force-pushed
the
fix-config-form-value
branch
from
July 15, 2026 11:06
932b316 to
0ddfc99
Compare
lippserd
force-pushed
the
fix-config-form-value
branch
from
July 16, 2026 12:13
0ddfc99 to
94384e3
Compare
TheSyscall
force-pushed
the
fix-config-form-value
branch
2 times, most recently
from
July 17, 2026 12:49
a48aaba to
2ae7c28
Compare
TheSyscall
changed the title
This is mainly done to catch a point before ON_ELEMENT_REGISTERED is called In `ConfigForm`s `ON_ELEMENT_REGISTERED` handler, the early return checked `$element->hasValue(`) in addition to `$key === null`. This meant that if an element already had a value (e.g. a default set by the element itself), the handler would skip seeding it with the config value entirely, and skip setting value candidates. - Dropped the `hasValue()` check from the early-return condition, so the config value is always considered for a keyed element, regardless of any pre-existing default. - On initial render (no value candidates yet), replaced the direct `$element->setValue($configValue)` call with `$this->populate([$element->getName() => $configValue])`, so the value also becomes visible via `getPopulatedValue()`, not just on the element instance itself.
The old `registerElement` implementation read the submitted value via `getPopulatedValue`, cleared the queue, pushed the config value, then pushed the submitted value back. This dropped any intermediate populate calls made before assembly and never put the config value into `ValueCandidates`, which broke `PasswordElement`: submitting `DUMMYPASSWORD` could not be resolved back to the stored password because the real value was missing from the candidates list. Since `populatedValues` is now protected on the `FormElements` trait, we can prepend the config value directly to the queue before delegating to `parent::registerElement`. The parent then applies the last queued value as the element's current value and passes the full queue to `setValueCandidates` — which now includes the config value at the front, giving `PasswordElement` the history it needs to resolve dummy passwords correctly. A `stdClass` sentinel replaces `null` as the `Config::get` fallback so that a config key explicitly absent from the file is distinguishable from one that holds a `null` value, avoiding a spurious prepend in that case.
Use the element's default as the `Config::get` fallback in `ConfigForm` Previously `registerElement` passed no fallback to `Config::get`, so the config value was always `null` when the key was absent. This caused elements with declared defaults to lose their value: the element attribute was set, but the populate queue remained empty, leaving `getPopulatedValue` returning `null`. Passing `$this->originalValues[$name]` as the fallback preserves the element's declared default when no config key exists. The check that decides whether to re-apply the submitted value over the config value is also tightened from `!== null` to `! Str::isEmpty` so that empty and whitespace-only submitted values are not pushed back onto the queue.
TheSyscall
force-pushed
the
fix-config-form-value
branch
from
July 17, 2026 12:53
2ae7c28 to
5502979
Compare
TheSyscall
marked this pull request as ready for review
July 17, 2026 13:12
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
2 participants
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.
ConfigForm's ON_ELEMENT_REGISTERED handler skipped seeding an element with its config value whenever the element already had some value set (
$element->hasValue()), even on initial render. When it did seed the value, it called$element->setValue()directly instead of$this->populate(), so the value was visible on the element but not throughgetPopulatedValue().This broke forms that read defaults via getPopulatedValue() during rendering, most notably CspConfigForm, where checkboxes ended up rendering with the wrong checked state.
Changes
ConfigForm: No longer skip populating the value of form elements when the element, has a default value.ConfigForm: on initial render, populate the form with the config value instead of setting it only on the element, sogetPopulatedValue()reflects it too.CspConfigForm/ConfigController: now thatConfigFormhandles this correctly, remove the workaround that manually computed and populated default values for the CSP enable checkboxes in the controller, and simplify form construction into a fluent chain.Requires
FormElements::$populatedValuesprotected ipl-html#204