[DomCrawler] check for the correct field type #11692
Conversation
| // there may be other fields with the same name that are no choice | ||
| // fields already registered | ||
| // see https://github.com/symfony/symfony/issues/11689 | ||
| if ($this->get($node->getAttribute('name')) instanceof ChoiceFormField) { |
There was a problem hiding this comment.
for consistency with other cases, this condition should be moved inside the previous if, so that the ChoiceFormField is added in other cases, for consistency with other cases where the same name appears several times
There was a problem hiding this comment.
Are you sure that this doesn't lead to any unexpected behaviour?
There was a problem hiding this comment.
well, if you have 2 text inputs with the same name, the second one wins. If you have a radio button and then a select with the same name, the second one (the select) wins. But in your code, if you have a field followed by a radio with the same name, the first field will win. This is inconsistent. we should always make the field overwrite the previous field for consistency (which also means that in the case of the form shown in your PR description, the radio button will be available rather than the hidden input)
|
👍 (the test failure is a volatile test in the Process component) |
HTML allow to define different form fields with the same name.
Imagine the following form:
<html>
<body>
<form action="/">
<input type="hidden" name="option" value="default">
<input type="radio" name="option" value="A">
<input type="radio" name="option" value="B">
<input type="hidden" name="settings[1]" value="0">
<input type="checkbox" name="settings[1]" value="1" id="setting-1">
<button>klickme</button>
</form>
</body>
</html>
Since the `FormFieldRegistry` can only handle one field per name, the
hidden field option is registered first before the radio field with
the same name is evaluated. Thus, the `FormFieldRegistry` returns an
`InputFormField` instance on which the `addChoices()` method can not
be called.
|
👍 |
|
Thanks for fixing this bug @xabbuh. |
This PR was merged into the 2.3 branch. Discussion ---------- [DomCrawler] check for the correct field type | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #11689 | License | MIT | Doc PR | HTML allow to define different form fields with the same name. Imagine the following form: ```html <html> <body> <form action="/"> <input type="hidden" name="option" value="default"> <input type="radio" name="option" value="A"> <input type="radio" name="option" value="B"> <input type="hidden" name="settings[1]" value="0"> <input type="checkbox" name="settings[1]" value="1" id="setting-1"> <button>klickme</button> </form> </body> </html> ``` Since the `FormFieldRegistry` can only handle one field per name, the hidden field option is registered first before the radio field with the same name is evaluated. Thus, the `FormFieldRegistry` returns an `InputFormField` instance on which the `addChoices()` method can not be called. Commits ------- 169b397 check for the correct field type
HTML allow to define different form fields with the same name. Imagine the following form:
Since the
FormFieldRegistrycan only handle one field per name, the hidden field option is registered first before the radio field with the same name is evaluated. Thus, theFormFieldRegistryreturns anInputFormFieldinstance on which theaddChoices()method can not be called.