Skip to content

Commit fc597e3

Browse files
committed
[LiveComponent] Fix disabled choices being submitted by live form updates
1 parent 9f6fd1b commit fc597e3

7 files changed

Lines changed: 138 additions & 2 deletions

File tree

src/LiveComponent/assets/dist/live_controller.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ function getValueFromElement(element, valueStore) {
291291
function setValueOnElement(element, value) {
292292
if (element instanceof HTMLInputElement) {
293293
if (element.type === "file") return;
294+
if ((element.type === "radio" || element.type === "checkbox") && element.disabled) return;
294295
if (element.type === "radio") {
295296
element.checked = element.value == value;
296297
return;

src/LiveComponent/assets/src/dom_utils.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ export function setValueOnElement(element: HTMLElement, value: any): void {
7777
return;
7878
}
7979

80+
// a user cannot change a disabled checkbox or radio and browsers never
81+
// submit it: the server-rendered state is the source of truth, keep it
82+
if ((element.type === 'radio' || element.type === 'checkbox') && element.disabled) {
83+
return;
84+
}
85+
8086
if (element.type === 'radio') {
8187
// biome-ignore lint/suspicious/noDoubleEquals: need fuzzy matching
8288
element.checked = element.value == value;

src/LiveComponent/assets/test/unit/dom_utils.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,28 @@ describe('setValueOnElement', () => {
203203
expect(input.checked).toBeFalsy();
204204
});
205205

206+
it('Leaves a disabled checkbox alone: browsers never submit it', () => {
207+
const input = document.createElement('input');
208+
input.type = 'checkbox';
209+
input.checked = true;
210+
input.disabled = true;
211+
input.value = 'the_checkbox_value';
212+
213+
setValueOnElement(input, ['other_value']);
214+
expect(input.checked).toBeTruthy();
215+
});
216+
217+
it('Leaves a disabled radio alone: browsers never submit it', () => {
218+
const input = document.createElement('input');
219+
input.type = 'radio';
220+
input.checked = true;
221+
input.disabled = true;
222+
input.value = 'the_radio_value';
223+
224+
setValueOnElement(input, 'other_value');
225+
expect(input.checked).toBeTruthy();
226+
});
227+
206228
it('Sets data onto select multiple', () => {
207229
const select = document.createElement('select');
208230
select.multiple = true;

src/LiveComponent/src/ComponentWithFormTrait.php

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,9 +265,33 @@ private function extractFormValues(FormView $formView): array
265265
continue;
266266
}
267267

268-
// <input type="checkbox">
268+
// <input type="checkbox"> - Simulate browser behavior
269+
// Browsers never submit disabled controls, so a checked but
270+
// disabled checkbox is treated as unchecked.
269271
if (\array_key_exists('checked', $child->vars)) {
270-
$values[$name] = $child->vars['checked'] ? $child->vars['value'] : null;
272+
$values[$name] = $child->vars['checked'] && !self::isFieldDisabled($child) ? $child->vars['value'] : null;
273+
continue;
274+
}
275+
276+
// Expanded ChoiceType - Simulate browser behavior
277+
// The "value" already aggregates the checked checkboxes/radios,
278+
// but browsers never submit disabled controls, so the values of
279+
// disabled choices are dropped.
280+
if ($child->vars['expanded'] ?? false) {
281+
$disabledValues = [];
282+
foreach ($child->children as $expandedChild) {
283+
if (self::isFieldDisabled($expandedChild)) {
284+
$disabledValues[] = $expandedChild->vars['value'];
285+
}
286+
}
287+
288+
$value = $child->vars['value'];
289+
if ($disabledValues) {
290+
$value = \is_array($value)
291+
? array_values(array_diff($value, $disabledValues))
292+
: (\in_array($value, $disabledValues, true) ? '' : $value);
293+
}
294+
$values[$name] = $value;
271295
continue;
272296
}
273297

@@ -308,6 +332,16 @@ private function extractFormValues(FormView $formView): array
308332
return $values;
309333
}
310334

335+
/**
336+
* A field can be disabled at the form level (the "disabled" option) or
337+
* only in HTML (a "disabled" attribute, e.g. set through "choice_attr"):
338+
* browsers do not submit the control in either case.
339+
*/
340+
private static function isFieldDisabled(FormView $view): bool
341+
{
342+
return ($view->vars['disabled'] ?? false) || ($view->vars['attr']['disabled'] ?? false);
343+
}
344+
311345
private function clearErrorsForNonValidatedFields(FormInterface $form, string $currentPath = ''): void
312346
{
313347
if ($form instanceof ClearableErrorsInterface && (!$currentPath || !\in_array($currentPath, $this->validatedFields, true))) {

src/LiveComponent/tests/Fixtures/Form/FormWithManyDifferentFieldsType.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,23 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
121121
'expanded' => true,
122122
'multiple' => true,
123123
])
124+
->add('choice_multiple_disabled', ChoiceType::class, [
125+
'choices' => [
126+
'foo' => 1,
127+
'bar' => 2,
128+
],
129+
'expanded' => true,
130+
'multiple' => true,
131+
'choice_attr' => static fn ($choice) => 1 === $choice ? ['disabled' => true] : [],
132+
])
133+
->add('choice_expanded_disabled', ChoiceType::class, [
134+
'choices' => [
135+
'foo' => 1,
136+
'bar' => 2,
137+
],
138+
'expanded' => true,
139+
'choice_attr' => static fn ($choice) => 1 === $choice ? ['disabled' => true] : [],
140+
])
124141
->add('select_multiple', ChoiceType::class, [
125142
'choices' => [
126143
'foo' => 1,
@@ -134,6 +151,9 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
134151
])
135152
->add('checkbox', CheckboxType::class)
136153
->add('checkbox_checked', CheckboxType::class)
154+
->add('checkbox_checked_disabled', CheckboxType::class, [
155+
'disabled' => true,
156+
])
137157
->add('file', FileType::class)
138158
->add('hidden', HiddenType::class)
139159
->add('complexType', ComplexFieldType::class)

src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,13 @@ public function testHandleCheckboxChanges()
192192
'choice_required_with_empty_preferred_choices' => 'ok',
193193
'choice_expanded' => '',
194194
'choice_multiple' => ['2'],
195+
'choice_multiple_disabled' => [],
196+
'choice_expanded_disabled' => '',
195197
'select_multiple' => ['2'],
196198
'entity' => (string) $id,
197199
'checkbox' => null,
198200
'checkbox_checked' => '1',
201+
'checkbox_checked_disabled' => null,
199202
'file' => '',
200203
'hidden' => '',
201204
'complexType' => [
@@ -294,6 +297,48 @@ public function testHandleCheckboxChanges()
294297
;
295298
}
296299

300+
public function testDisabledChoicesAreNeverSubmitted()
301+
{
302+
CategoryFixtureEntityFactory::createMany(5);
303+
304+
$mounted = $this->mountComponent(
305+
'form_with_many_different_fields_type',
306+
[
307+
'initialData' => [
308+
// "foo" (value 1) is disabled through "choice_attr"
309+
'choice_multiple_disabled' => [1, 2],
310+
],
311+
]
312+
);
313+
314+
$dehydratedProps = $this->dehydrateComponent($mounted)->getProps();
315+
316+
// like a browser, the checked but disabled choice is not part of the
317+
// values that would be submitted
318+
$this->assertSame(['2'], $dehydratedProps['form']['choice_multiple_disabled']);
319+
320+
// a model update must not resurrect the disabled value either
321+
$crawler = $this->browser()
322+
->throwExceptions()
323+
->post('/_components/form_with_many_different_fields_type', [
324+
'body' => [
325+
'data' => json_encode([
326+
'props' => $dehydratedProps,
327+
'updated' => ['form' => ['choice_multiple_disabled' => []]],
328+
]),
329+
],
330+
])
331+
->assertSuccessful()
332+
->crawler()
333+
;
334+
335+
$dehydratedProps = json_decode(
336+
$crawler->filter('div')->first()->attr('data-live-props-value'),
337+
true
338+
);
339+
$this->assertSame([], $dehydratedProps['form']['choice_multiple_disabled']);
340+
}
341+
297342
public function testLiveCollectionTypeAddButtonsByDefault()
298343
{
299344
$dehydrated = $this->dehydrateComponent($this->mountComponent('form_with_live_collection_type'))->getProps();

src/LiveComponent/tests/Unit/Form/ComponentWithFormTest.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ public function testFormValues()
3434
$component = new FormComponentWithManyDifferentFieldsType($formFactory);
3535
$component->initialData = [
3636
'choice_multiple' => [2],
37+
'choice_multiple_disabled' => [1, 2],
38+
'choice_expanded_disabled' => 1,
3739
'select_multiple' => [2],
3840
'checkbox_checked' => true,
41+
'checkbox_checked_disabled' => true,
3942
];
4043
$component->initializeForm([]);
4144

@@ -54,10 +57,15 @@ public function testFormValues()
5457
'choice_required_with_empty_preferred_choices' => 'ok',
5558
'choice_expanded' => '',
5659
'choice_multiple' => ['2'],
60+
// disabled choices are never submitted by browsers, so their
61+
// values must not appear here even when initially selected
62+
'choice_multiple_disabled' => ['2'],
63+
'choice_expanded_disabled' => '',
5764
'select_multiple' => ['2'],
5865
'entity' => (string) $id,
5966
'checkbox' => null,
6067
'checkbox_checked' => '1',
68+
'checkbox_checked_disabled' => null,
6169
'file' => '',
6270
'hidden' => '',
6371
'complexType' => [

0 commit comments

Comments
 (0)