Leads to a WSOD when trying to submit the form with invalid value. Exception is not caught, since the validation is only run on custom language addition and not editing an existing language.
Add a language and use "llama" as the formula. WSOD happens.
1. Ensure validate handler is added to form submit in edit form (the form build array is different on addition and edit)
2. Change test in patch #2 so it submits "llama" as the plural formula.
3. Assert it gives the appropriate message on adding and editing a language.
Patch needed
Validation happens on edit and no WSOD.
None.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 3274208-validate-plural-formula-8.patch | 9.8 KB | penyaskito |
| #2 | 3274208.patch | 1.67 KB | gábor hojtsy |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
gábor hojtsyComment #3
gábor hojtsyThis would be how the test could look like at least for the exception case. I don!t know the empty array case would be, that would needto be looked up, when is that produced. But this should help with avoiding the WSOD.
Comment #5
penyaskitoI think this is a good task for the sprints.
There are several issues to consider:
1. As Gábor said, validation is not triggered in edit. We need to fix that. The reason is not triggered is that we are adding the validate handler a submit, but the structure for the edit form is different than the add form.
2. We need to modify the test or improve the parsing. The current parsing is actually happy with
nplurals=llama; plural=(n != 1);. Ideally we would be improving the parsing so we ensure nplurals is a number, but that's not triggering any failures at the moment. Drupal core is not validating that at the moment.3. We need a new test as the test Gábor added, where we submit plurals as some non-formula string (e.g. "llamas"). That will produce the NULL array, which would trigger the uncatched error in
Comment #6
gábor hojtsy@penyaskito: we can also scope this different and only fix the lack of validation running here and expand on the validation elsewhere, unless they cause the same sympthoms. Just to make sure we can get in each fix ASAP :)
Comment #7
penyaskitoCreated #3311420: Validate number of plurals in plural formula when adding/editing a language for that.
Adjusted issue summary with instructions for the new smaller scope.
Comment #8
penyaskitoAdded the #validate to the edit form.
For making it easier I separated add and edit forms, but we can revert that if not desired, as it changes the services definitions.
TIL #entity_builders runs before #validate handlers, so we need to control there that validation already happened.
Comment #9
diegorsI will review it.
Comment #10
diegorsThe patch works fine, when we add
llamaas formula before the patch we get the WSOD, after the patch, we get the error on form page, also works in the edit form. Moving to RTBC.Comment #13
gábor hojtsyComment #14
gábor hojtsyDue to a mismatch in when I checked the credit, the commit credit did not land with @penyaskito and @diegors unfortunately, sorry. The issue credit was properly set though.