I'm not sure how this can happen, but in #1045208: Form altering of node form doesn't happen with multiselect select form and #ajax I discovered a situation where $form_state['clicked_button'] is not set, even though $form_state['triggering_element'] is properly set.
Since $form_state['clicked_button'] is obsolete, let's get rid of references to it in core. This does *not* remove the assignment of $form_state['clicked_button'], so it will work just as well or as poorly with this patch.
The place where $form_state['clicked_button'] was not set (in a form-altered #ajax addition to the form) was
function file_managed_file_validate(&$element, &$form_state) {
// 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['clicked_button']['#parents']);
Comment | File | Size | Author |
---|---|---|---|
#35 | Selection_027.png | 21.08 KB | emmajane |
#31 | drupal.lose_clicked_button.patch | 6.68 KB | fago |
drupal.lose_clicked_button.patch | 6.67 KB | rfay | |
Comments
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis is a duplicate of #1031412: File field assumes that the form is always submitted by pressing a button.
Comment #3
rfayI'd like to go the other way, if we can, as this issue talks about the actual cause of the other issues.
Marked #990260: create a new book from non-book content type gives "Passed variable is not an array..." error. as a duplicate.
Marked #1031412: File field assumes that the form is always submitted by pressing a button as a duplicate.
Comment #4
droplet CreditAttribution: droplet commentedsub from #990260
Comment #5
timhilliard CreditAttribution: timhilliard commentedAny variables that have button in their name should probably also be changed (ie. $clicked_button -> $triggering_element).
Comment #6
rfay@timhilliard, I disagree with that. It's more intrusive, more likely to create bugs, and there's nothing wrong with the variable being named "button".
Comment #7
elamanPatch fixed only "Warning: end(): Passed variable is not an array or object i file_managed_file_validate() (linje 536 af /home/www/demosite.oldrup.dk/modules/file/file.module)." error. Still get Illegal choice error.
Comment #8
rfay@Peritus, your "illegal choice" issue is #1045208: Form altering of node form doesn't happen with multiselect select form and #ajax. There is not yet a patch for it. I don't believe it's related to this one. You just happened to have both on that same page.
Comment #9
effulgentsia CreditAttribution: effulgentsia commentedsubscribing to look at later
Comment #10
ogi CreditAttribution: ogi commentedsubscribe
Comment #11
rfaydrupal.lose_clicked_button.patch queued for re-testing.
Comment #12
elamanpatch worked for me
Comment #13
fagoI just ran over the same problem. The cause is that 'clicked_button' is only set for buttons but not for #ajax submits triggered by other elements, thus any handlers on a form that always run (such as the file validation handler) should use 'triggering_element'.
Also, as 'clicked_button' is deprecated by 'triggering_element' anyway, converting all uses in core makes much sense. Thus #1 is great, and also fixes the problem I ran over.
Comment #14
bfroehle CreditAttribution: bfroehle commentedJust marked #1044264: Book/Forum Warning: end() expects parameter as a duplicate.
Comment #15
bazzly CreditAttribution: bazzly commentedCan someone tell me how I can apply this patch...what directory do I need to drop it in?
Thanks
Comment #16
bfroehle CreditAttribution: bfroehle commentedbazzly: http://drupal.org/patch/apply
Comment #17
bazzly CreditAttribution: bazzly commentedThanks
Comment #18
effulgentsia CreditAttribution: effulgentsia commentedNow that the D8 branch is open, this needs to go in there first, then be backported to D7.
Comment #19
effulgentsia CreditAttribution: effulgentsia commentedRetitling to clarify that this is a bug fix, not a task :)
As per #13, the bug fix is only needed in the validation functions, not in the submit functions (since non-buttons don't trigger submit handlers), but at least for D8, we should clean this up everywhere, as #1 does, so RTBC+1. When deciding what to backport, we can discuss whether to do all of it, or only the ones strictly needed to fix the bugs.
Comment #20
longwaveSubscribe, ran into the error message in #7 when trying to add an Ajax select to a node form.
Comment #21
timhilliard CreditAttribution: timhilliard commented@effulgentsia: as per post #990260: create a new book from non-book content type gives "Passed variable is not an array..." error. which is already referenced in comment #3, there are ways to trigger a submit other than a button. The book type triggers a submit using the select form element and onchange event handler.
Comment #22
shift31 CreditAttribution: shift31 commentedsubscribing...can the above patch be safely applied to D7? I modified file.module accordingly for now...thank you.
Comment #23
rfay@shift31, yes this is a D7 patch. Just has to be applied to 8.x first.
Comment #24
rfaydrupal.lose_clicked_button.patch queued for re-testing.
Comment #25
arlinsandbulte CreditAttribution: arlinsandbulte commentedTagging to backport to D7 to resolve this duplicate issue:
#990260: create a new book from non-book content type gives "Passed variable is not an array..." error.
Comment #26
eric.chenchao CreditAttribution: eric.chenchao commentedThanks, this patch works.
Comment #27
localhost CreditAttribution: localhost commentedsubscribe. works for me too.
Comment #28
redndahead CreditAttribution: redndahead commenteddrupal.lose_clicked_button.patch queued for re-testing.
Comment #30
longwaveMarked #1122014: Warning: end() expects parameter 1 to be array, null given in file.module line 536 as duplicate
Comment #31
fagore-rolled patch with Git (-p1).
Comment #32
Kisugi Ai CreditAttribution: Kisugi Ai commentedDosen't work realy
if used this pacht the ajax wont work anymore corectly
Comment #33
Patrick R. CreditAttribution: Patrick R. commentedSubscribing.
Comment #34
Kisugi Ai CreditAttribution: Kisugi Ai commentedhttp://drupal.org/node/990260#comment-3891880
works
Comment #35
emmajane CreditAttribution: emmajane commentedSubscribe. It'd be great to see this rolled to out to D7 sooner rather than later.....
Comment #36
Kisugi Ai CreditAttribution: Kisugi Ai commentedhttp://drupal.org/node/990260#comment-3891880
take this pach its work
Comment #37
cossovich CreditAttribution: cossovich commentedsubscribing.
Comment #38
davidbarratt CreditAttribution: davidbarratt commentedsubscribe.
Comment #39
longwaveResetting version and assigned fields.
Comment #40
priteshrn CreditAttribution: priteshrn commenteddrupal.lose_clicked_button.patch queued for re-testing.
Comment #41
priteshrn CreditAttribution: priteshrn commentedComment #42
priteshrn CreditAttribution: priteshrn commented#31: drupal.lose_clicked_button.patch queued for re-testing.
Comment #43
bfroehle CreditAttribution: bfroehle commentedComment #44
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe sooner this type of bug fixes get in, the better. Or people start adding crazy and crappy workarounds everywhere in contributed and specific modules.
Let's get this in, with or without tests.
Comment #45
rfay#31: drupal.lose_clicked_button.patch queued for re-testing.
Comment #46
sun+1, 'clicked_button' is documented as deprecated already, so Drupal core should lead by example and don't use it anywhere.
Comment #47
sunCreated #1204250: $form_state['clicked_button'] is deprecated for Coder module.
Comment #48
webchickLooks like this fixes a good chunk of issues. Should probably be "major" to give it a bit more visibility.
Committed to 8.x and 7.x. Thanks!
However:
"+1, 'clicked_button' is documented as deprecated already, so Drupal core should lead by example and don't use it anywhere."
That doesn't actually seem to be true, according to http://drupal.org/update/modules/6/7. Tagging "Needs Update Documentation". sun, it'd be helpful if you could write up a blurb to take care of this.
Comment #49
sunIt's somewhat documented in the Form API topic: http://api.drupal.org/api/drupal/includes--form.inc/group/form_api/7
Added http://drupal.org/update/modules/6/7#clicked_button
Comment #50
jcook4now CreditAttribution: jcook4now commentedWebchick, Although you said that this was committed to 7.x, this does not seem to be fixed for me in 7.4 - I'm still getting:
Warning: end() expects parameter 1 to be array, null given in file_managed_file_validate() (line 536 of \modules\file\file.module). . .
Jeff
Comment #51
webchickYeah, this fix won't be available until Drupal 7.5. Or you could download the 7.x-dev release.
Comment #52
jcook4now CreditAttribution: jcook4now commentedOK -Thanks for clarifying. . .
Comment #54
hanoiiI was referred to this issue by following #1213574: Ajax Cart not working with uc_out_of_stock notification which is a bug related to a an AJAX call in which clicked_button is not sent. What's the proper/best way for a D6 module to add clicked button as it would have been a normal click?
Comment #55
rfayThis issue is specifically about D7/D8 changing away from $form_state['clicked_button']. You should continue your conversation in your original issue.
Comment #56
fortis CreditAttribution: fortis commentedI have this problem only with enabled views_megarow-7.x-1.x-dev (2013-Aug-10)
Comment #57
lsolesen CreditAttribution: lsolesen commented@fortis I referenced this in issue #2112071: Warning: end(): Passed variable is not an array or object i file_managed_file_validate() opened in views_megarow issue queue.