Problem/Motivation
When programmatically submitting forms, I noticed the logs filling up with the following warning:
Warning: end() expects parameter 1 to be array, null given in Drupal\file\Element\ManagedFile::validateManagedFile() (line 404 of
/core/modules/file/src/Element/ManagedFile.php)
This is because of the following code:
/**
* Render API callback: Validates the managed_file element.
*/
public static function validateManagedFile(&$element, FormStateInterface $form_state, &$complete_form) {
// If referencing an existing file, only allow if there are existing
// references. This prevents unmanaged files from being deleted if this
// item were to be deleted.
$clicked_button = end($form_state->getTriggeringElement()['#parents']);
According to the FormStateInterface, a valid return value for getTriggeringElement() is null:
/**
* Gets the form element that triggered submission.
*
* @return array|null
* The form element that triggered submission, of NULL if there is none.
*/
public function &getTriggeringElement();
In the case of my programmatic submission, the call to getTriggeringElement() returns null (again, this is valid according to the interface), which is then passed to php's end function, which expects an array.
One potential solution is below (patch to come):
$triggering_element = $form_state->getTriggeringElement();
$clicked_button = $triggering_element ? end($triggering_element['#parents']) : '';
Steps to reproduce
- Install and enable webform 8.x-5.0-beta13
- Create a webform with an element for attachments (i.e. managed_file element)
- Write code to submit a webform programmatically as seen in the webform cookbook.
- Submit the form programmatically
Note: This should happen with any programmatic form submission with a managed_file element, my entry point just happened to be through the webform module.
Expected behavior:
- Submission validated or created without PHP warnings.
What happened instead:
- PHP warning described below observed.
This issue was observed on:
- macOS Sierra 10.12.2
- Drupal 8.3.7 instance of Core, but see that it still exists in 8.5.x.
Proposed resolution
Remaining tasks
Add a test
review
commit
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | interdiff_17-21.txt | 2.53 KB | murilohp |
| #21 | 2910320-21.patch | 2.46 KB | murilohp |
| #17 | 2910320-17.patch | 2.78 KB | murilohp |
| #17 | 2910320-test-only.patch | 1.89 KB | murilohp |
| #2 | validating_managed-2910320-2.patch | 891 bytes | malik.kotob |
Comments
Comment #2
malik.kotob commentedComment #3
malik.kotob commentedComment #4
malik.kotob commentedPatch for 8.3.7 attached.
Comment #5
barrett commentedI've reviewed and can confirm both the originally reported issue and that the attached patch resolves the issue.
Comment #6
gábor hojtsyThis seems very logical that it would fix the issue. Is there a way to add tests to it though?
Comment #7
henk commentedI wrote the custom method to submit form via API and had same problem with saving files, for me fix was adding to submit form argument:
Patch check only if triggering element exist, probably we should check if it's array with key '#parents'.
I think it's design question do we force to use Triggering Element for any form (documentation should be updated) or allow exeption in some case (code must be updated).
Comment #16
quietone commentedThis was a bugsmash target today. The changes in the patch haven't been added to core so I guess this is still valid and still needs tests. I did not test.
Comment #17
murilohp commentedI've added a test in this new patch and also followed #7 suggestion, now the
#parentsis validated and this should avoid the error.The test-only patch is supposed to be failed with the same error provided on the IS.
Comment #18
murilohp commentedSince the test patch failed with the correct error, I'm moving to NR.
Comment #19
henk commentedTest for 2910320-17.patch probably faild on CKeditor problem from here: https://www.drupal.org/project/drupal/issues/3268368
Checking if array key "parents" exist should fix the PHP error problem.
Comment #20
larowlanWe can have the test and form as the one class (no need for the extra class) https://www.previousnext.com.au/blog/drupal-8-ftw-it-test-or-it-form-act...
The test failure is Media library, not CKEditor, which might be related to this patch, can we get a re-run please?
Comment #21
murilohp commentedHey @larowlan, thanks for your input here, I've changed the test and put the form inside the test class. Awesome blog and great post by the way! Really helpful!
Regarding the failed test, I wasn't able to reproduce it locally, it's passing here :( , so let's take another shot.
Comment #22
jonathanshawLooks good.
Comment #24
larowlanConfirming the test fails without the change
Comment #29
larowlanCommitted to 10.1.x and backported to 10.0.x and 9.5.x - thanks!