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']);

Comments

Status: Needs review » Needs work

The last submitted patch, drupal.lose_clicked_button.patch, failed testing.

damien tournoud’s picture

Status: Needs work » Closed (duplicate)
rfay’s picture

Status: Closed (duplicate) » Needs review

I'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.

droplet’s picture

sub from #990260

timhilliard’s picture

Any variables that have button in their name should probably also be changed (ie. $clicked_button -> $triggering_element).

rfay’s picture

@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".

elaman’s picture

Patch 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.

rfay’s picture

@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.

effulgentsia’s picture

subscribing to look at later

ogi’s picture

subscribe

rfay’s picture

drupal.lose_clicked_button.patch queued for re-testing.

elaman’s picture

patch worked for me

fago’s picture

Status: Needs review » Reviewed & tested by the community

I 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.

bfroehle’s picture

bazzly’s picture

Can someone tell me how I can apply this patch...what directory do I need to drop it in?
Thanks

bfroehle’s picture

bazzly’s picture

Thanks

effulgentsia’s picture

Title: $form_state['clicked_button'] is not always set, so we should just use $form_state['triggering_element'] » Remove usage of deprecated $form_state['clicked_button']
Version: 7.x-dev » 8.x-dev

Now that the D8 branch is open, this needs to go in there first, then be backported to D7.

effulgentsia’s picture

Title: Remove usage of deprecated $form_state['clicked_button'] » Usage of deprecated $form_state['clicked_button'] causes bugs during AJAX submissions by non-buttons

Retitling 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.

longwave’s picture

Subscribe, ran into the error message in #7 when trying to add an Ajax select to a node form.

timhilliard’s picture

@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.

shift31’s picture

subscribing...can the above patch be safely applied to D7? I modified file.module accordingly for now...thank you.

rfay’s picture

@shift31, yes this is a D7 patch. Just has to be applied to 8.x first.

rfay’s picture

drupal.lose_clicked_button.patch queued for re-testing.

arlinsandbulte’s picture

eric.chenchao’s picture

Thanks, this patch works.

localhost’s picture

subscribe. works for me too.

redndahead’s picture

Issue tags: -Needs backport to D7

drupal.lose_clicked_button.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, drupal.lose_clicked_button.patch, failed testing.

longwave’s picture

fago’s picture

Status: Needs work » Needs review
StatusFileSize
new6.68 KB

re-rolled patch with Git (-p1).

kisugiai’s picture

Dosen't work realy
if used this pacht the ajax wont work anymore corectly

patrick r.’s picture

Subscribing.

kisugiai’s picture

emmajane’s picture

StatusFileSize
new21.08 KB

Subscribe. It'd be great to see this rolled to out to D7 sooner rather than later.....

kisugiai’s picture

cossovich’s picture

subscribing.

davidbarratt’s picture

Version: 8.x-dev » 7.2
Assigned: Unassigned » davidbarratt

subscribe.

longwave’s picture

Version: 7.2 » 8.x-dev
Assigned: davidbarratt » Unassigned

Resetting version and assigned fields.

priteshrn’s picture

drupal.lose_clicked_button.patch queued for re-testing.

priteshrn’s picture

Version: 8.x-dev » 7.2
priteshrn’s picture

#31: drupal.lose_clicked_button.patch queued for re-testing.

bfroehle’s picture

Version: 7.2 » 8.x-dev
damien tournoud’s picture

Status: Needs review » Reviewed & tested by the community

The 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.

rfay’s picture

#31: drupal.lose_clicked_button.patch queued for re-testing.

sun’s picture

+1, 'clicked_button' is documented as deprecated already, so Drupal core should lead by example and don't use it anywhere.

sun’s picture

webchick’s picture

Priority: Normal » Major
Status: Reviewed & tested by the community » Fixed
Issue tags: +Needs documentation updates

Looks 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.

sun’s picture

jcook4now’s picture

Webchick, 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

webchick’s picture

Yeah, this fix won't be available until Drupal 7.5. Or you could download the 7.x-dev release.

jcook4now’s picture

OK -Thanks for clarifying. . .

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

hanoii’s picture

I 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?

rfay’s picture

This issue is specifically about D7/D8 changing away from $form_state['clicked_button']. You should continue your conversation in your original issue.

fortis’s picture

I have this problem only with enabled views_megarow-7.x-1.x-dev (2013-Aug-10)

lsolesen’s picture

@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.