Notice: Undefined index: process_input in _form_builder_handle_input_element() (line 1162 of [..]/drupal7-dev/includes/form.inc).
Notice: Undefined index: complete form in form_builder() (line 1052 of [..]/drupal7-dev/includes/form.inc).
I know it's just an E_ALL notice, but isn't Drupal core supposed to be clean of these?
Caused by:
- Clean D7 CVS checkout. PHP E_ALL error handling enabled on my dev machines.
- Enabled 'upload', created a content type, started making a node.
- Using 'File attachments', attached a jpeg (same results when attaching a txt)
- Used the AJAX "Attach" button
= Multiple versions of the above errors were displayed (in the AJAX response area)
Looks like the func in question _form_builder_handle_input_element()is expecting some things to be filled in on $form_state
if ($form_state['programmed'] || ($form_state['process_input'] && (!isset($element['#access']) || $element['#access']))) {
that have not been initialized. Probably a short-cut that the AJAX callback took? Could it be needing to add form_state_defaults()?
Avoiding the AJAX and just choosing the file and submitting the form without 'Attach' ing it did not show this notice.
Posting this against 'upload.module' not forms as the Drupal trend is to get whatever is triggering the problem to stop doing so (although I prefer putting assertions into APIs to sanitize bad input myself)
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 612396.patch | 869 bytes | jim0203 |
Comments
Comment #1
dman commentedOn investigation, only form_builder ever sets the expected value of
$form_state['process_input']- and that only on the top level of the form.upload_js (which is the ancestor of this error) seems to retrieve the form from the "Form API cache". Then does ... something ... before calling form_builder() ... which doesn't like that input.
Guessing again, it's something about the way upload_js processes only a sub-element of the form, but form builder is assuming it's a top-level form element that usually has more attributes.
I'll stop guessing now, as I'm just speculating and probably going off-course.
Comment #2
jim0203 commented@dman I've stepped through this whole process and it looks like your guess is correct.
upload_js() retries the form from cache, messes with it in various ways and then calls form_builder(). Most of the things upload_js() does to the form don't seem too important in the context of this issue, but on line 695 it does (re)set $form_state:
All the hook_form_alter() functions are then called, and then form_builder() is called. However, as you state, form_builder() implicitly expects to receive a form which has a top-level form element, and that's not what upload_js() is giving it. Because form_builder() never gets a top-level form element, it never sets either $form_state['process_input'] or $form_state['complete form'], which is what causes the errors in this issue. They're not serious and don't cause any problems beyond the error messages themselves, but they do need to be wiped out.
The attached patch just changes line 695 of upload.module to
This is the line that usually resets $form_state anyway, so there's not point changing anything before this point. The only thing that could (perhaps, technically) change $form_state (i.e., unset the values set by the patch) between line 695 and line 698 when form_builder() is called is some hook_form_alter() called by drupal_alter() on line 697. However, as it stands form_builder() is assuming that $form_state['submitted'] and $form_state['programmed'] are going to be set, so I think it's pretty low risk to do things this way.
Comment #3
dman commentedYeah, it looks like that's what is needed to make that happier. I saw the 'process_input', but seeing as I couldn't see why that was expected to be set or what it meant, I didn't know whether it was safe to fudge or not.
Your analysis looks good to me.
It's a bit of a cheat, but the whole AJAX-processing a sub-form is a cheat, so whatever it takes to fool the system is good.
+1 harmless from me :-)
Thanks for looking into that!
Comment #4
webchickThanks for the sleuthing! I committed this, but would /really/ welcome people helping out at #563000: Provide upgrade path to file so we can ditch upload module altogether in favour of the vastly-superior-in-every-way file module. :)