The setClick setting in the ajax system is meant to be a way to tell the ajaxSubmit function which button was actually clicked. It uses this code:

          ajax.form.clk = this.element;

The problem here is that ajax.form is actually a jQuery object, not the DOM object that is necessary to set the clk. A secondary bug is that 'this' at this point is the element, so this.element is not set.

There are two ways of fixing this.

1) since 'this' is the element that was clicked, we can just use:

  this.form.clk = this;

I'm pretty sure this is what this was in my original patches, and got changed somewhere along the way. In CTools for D6, that is exactly what I'm using. Why this got changed, I am not certain. I can't even be certain who made the change or if I made it, at this point, but that's the form that it got committed in.

2) We can go with indirection:

  ajax.form.get(0).clk = ajax.element;

This seems inelegant to me. Patch with solution #1 forthcoming.

CommentFileSizeAuthor
#1 932848-fix-setClick-no-really.patch634 bytesmerlinofchaos
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

Here's the patch.

rfay’s picture

Subscribe. Probably worth doing a git blame/cvs blame to see if this was changed in regard to some issue.

webchick’s picture

Status: Active » Needs review

Marking as a patch.

merlinofchaos’s picture

I already did the cvs blame and as I tried to say in my original post, this is the form that it went in with. But patches change a lot as they go through various people's hands, and I often find myself wondering why someone changed a piece of code in one of my patches. Just like I often find myself wondering why I changed a piece of code in my own patch. These things happen.

The point is:

$(form) != form (one is a jquery object, one is a DOM element). $(form).clk has no meaning where form.clk does. Therein lies the bug.

I have this working in Panels with this patch. In order to really test this, you need to ajax submit a form with two buttons using ctools-ajax-submit; before the patch, 'op' will not be sent so the first button will always appear to be clicked. With the patch the correct button will appear to submit.

ksenzee’s picture

Status: Needs review » Reviewed & tested by the community

Yeah, this is clearly a bug. To be honest I have a slight preference for your #2 even if it is inelegant, just because I find it hard to read code that relies on "this" in anything but an OO context. But that is nothing but a bikeshed. The patch in #1 is correct, it fixes the bug, and we can't write tests for it until we have a full JS test harness. So RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed #1 to HEAD. Thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix

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