AHAH/AJAX bindings do not work on checkbox or radio in IE6/7/8 (and now IE9)

Working through the same issue in D6. Found this:

http://opensourcery.com/blog/chad-granum/ie-onchange-events

It's a hack, but It might help.

Just come across this thread and can confirm that I'm facing this issue too.

For the benefit of anybody searching, the problem is that, in IE, the [#ajax] callback assigned to a checkbox does not fire until focus of that element is lost - i.e. you click off the checkbox again.
You can recreate this easily using the "AJAX Example: generate textfields" example from the "Examples for Developers" module (www.drupal.org/project/examples).
The example in question has a checkbox form element as follows:

  $form['ask_first_name'] = array(
    '#type' => 'checkbox',
    '#title' => t('Ask me my first name'),
    '#ajax' => array(
      'callback' => 'ajax_example_autotextfields_callback',
      'wrapper' => 'textfields',
      'effect' => 'fade',
    )
  );

Checking the checkbox "Ask me for my first name" results in no action (no throbber, nothing) in Internet Exporer. Clicking off the checkbox (onto another form element, or just onto an empty bit of page) causes the callback to fire.

However, I also find that callbacks can be lost if, for example, you check checkbox1, and then check checkbox2 (thereby causing callback1 to fire), callback2 never fires, even after you then click off checkbox2.

This pretty much kills the possibility of currently using AJAX checkboxes (although other form elements such as select boxes are unaffected)

Hi,
Here a tar with the Chad's hack rewritten in a little module. Just download it in your module folder and enable it. The bug is fixed in IE 7 and i think IE6 and IE8 too, but i still have to test.
I just took the original Chad's code and added in the js file a condition if(Drupal.jsEnabled){} and create module and info files to quick implementation in a drupal 6 installation.

I think reason of this issue being fixed is that since jQuery 1.4 "change" and "submit" events are being normalized (http://api.jquery.com/change/)

Hi
@rfay sorry i didn't mind this issue is about drupal 7 ! The micro module i posted is a drupal 6.x version. I would be glad to work a (very very) little bit on the drupal 7 version, so i can try to do a patch for it, and for the ahah.js file in the 6th version (i didn't find ajax.js file in the misc repository for the 6th)
But, i can't do it before monday. So on monday evening i try it and i send news here.
Sorry for my froggy english ;-) !

Ok, sorry again, we are on monday evening and a i didn't manage to find out only one minut to do it ! Actually, i'm really unfamiliar with patch, but i thk i can do it. Just give me time (four day more...)

Hi,
Ok, here a first patch. To be honest, i just make a diff and patch. I did not any test (no time, and i don't have virtual box ready at home, only linux and no IE to test).
So i can't promise anything about it.
If you want you can test it and maybe improve it...
I test soon
Bye

Ok,
Sorry for mistakes,
Do you want i remove the patch until i can test it ?

Hi,
sorry for the long time without news and work. I try something those week end.
Cheers
Timos

Hum,
I tried to do the job on d7, but i'm a litte bit surpised... the example module doesn't work even in firefox ! So i can't really test it. I'm not aware about the d7 API, so i can't find out why it doesn't work....
I'm sorry but i think my contrib will be limited on a tricky hack for D6.

thought this problem, i try to add the js lines to the ajax.js file and it seems it doesnt' do anything. I can see that as i test the module in ie too, and found the same behavior you note : the checkbox never stay checked even after a click (in my case the ajax event is not fired, but is not fired in firefox too - maybe the alpha 5 release change something...).
I try with and without the trick i mentioned above, but the behavior is always the same...

Maybe if you can explain why the ajax event never fire in the example module given in the description issue, i can test again...

Cheers

Timos

Ok, i tried with the example ajax module, and well, ajax event are fired but now i can say the hack whom i talked doesn't work anymore...

I can confirm that this patch worked for me. I had the excact same problem with MSIE not updating checkbox states when used in conjunction with #states.

I'm using alpha-6, so this issue is still occuring. Would be great if this patch could be applied.

Confirming that it works...
Quickfix after all...

I'm not sure it makes sense to return true from an event handler, where before we were returning false, without also paying attention to whether we want to prevent the default event and stop event propagation (both of which happen automatically when you return false). This will be the click handler for ajax links, for example, and we don't want the default event on those. Maybe we need conditionals to check what kind of event we're handling.

Right, that's exactly my point - allowing the default event in all cases is dangerous. Or at least it should be dangerous. It does seem odd that the submit buttons aren't breaking. I'll try to find time to step through and see if I can figure out why.

What if we had a element_settings.return_value?

diff --git misc/ajax.js misc/ajax.js
index 553d2cc..e8cf6c6 100644
--- misc/ajax.js
+++ misc/ajax.js
@@ -164,7 +164,7 @@ Drupal.ajax = function (base, element, element_settings) {
       $.ajax(options);
     }
 
-    return false;
+    return element_settings.return_value || false;
   });
 
   // If necessary, enable keyboard submission so that AJAX behaviors

Then for the checkboxes, we would pass element_settings.return_value = true.... Might be overkill, but hey.

It looks good to me. It might be fixing a symptom rather than a root cause, but since none of us can find a root cause, we should definitely fix the symptom.

+++ misc/ajax.js
@@ -164,7 +164,14 @@ Drupal.ajax = function (base, element, element_settings) {
+    if (this.type == 'checkbox' || this.type == 'radio') {

Given #647228: Links are needlessly unable to fully participate in D7 AJAX framework features, this.type will fail if this is not an INPUT element. Let's make sure it is one.

I wonder whether we shouldn't reverse this condition -- returning false actually only makes sense for buttons (forms) and links...?

Powered by Dreditor.

Are you saying that the propsed patch will fail at some point as 'this' might represent a link? If that is the case, could we just add another check like the one below?

if (this.tagName == 'INPUT' && (this.type == 'checkbox' || this.type == 'radio')) {

This issue still exists in Drupal 7 (beta 3), so it's probably a good idea to get this one done :-)

I did some quick searching and couldn't find a reference with a list of which DOM elements have default actions on click events (which is the little corner of JS in question). Since we're already returning true (i.e. allowing the default action and propagating the event) in most cases, let's continue to do so, and only return false (i.e. prevent the default action and stop event propagation) for elements we know to have problems already. That's the smallest possible change I can think of, which seems appropriate at this point in the cycle.

Committed to HEAD. Thanks!

I discovered this issue while working on #995854: #ajax doesn't work at all if a file element (or enctype => "multipart/form-data") is included in the form. Any chance we can change the logic a bit to this? In line with #23 and a D8 @todo for #27. While not a substantive change, I think it's a little more understandable, and gives modules better capability to customize.

@rfay: Can you clarify "does not bind correctly to checkboxes" ?

+++ misc/ajax.js	21 Dec 2010 02:15:21 -0000
@@ -111,7 +111,8 @@ Drupal.ajax = function (base, element, e
+    allow_default_action: false

We should consider to use a property name closer to standards -- isn't this called "bubbles" normally?

Powered by Dreditor.

It turns out the problem here is how IE handles change events on radios and checkboxes, which I now remember being bitten by in the past. Clearly I repressed the memory. From http://www.quirksmode.org/dom/events/change.html#t04 :

IE fires the event when the checkbox or radio is blurred, and not when it is activated. This is a serious bug that requires the user to take another action and prevents a consistent cross-browser interface based on the change event on checkboxes and radios.

So IE doesn't consider a box as checked until you've checked it *and then* clicked somewhere outside it. This is true up to and including IE9. The workaround is to use the click event instead, which works fine. I'm attaching a patch that does that.

I'm also attaching a version of the patch that rolls back the fix from #31. The point of that code was to special-case change events for radios and checkboxes, and we're no longer binding to those events. So in my opinion the special casing is now unnecessary. I would however like effulgentsia's take on whether the abstraction he proposed would still be useful in other contexts.

+++ includes/ajax.inc
@@ -587,12 +587,19 @@ function ajax_pre_render_element($element) {
+      case 'radio':
+      case 'checkbox':
       case 'link':
         $element['#ajax']['event'] = 'click';

Is the click event fired when toggling a radio/checkbox with the spacebar?

Powered by Dreditor.

Yep: http://www.quirksmode.org/dom/events/click.html#t05

+++ includes/ajax.inc
@@ -587,12 +587,19 @@ function ajax_pre_render_element($element) {
+      // Note on 'change' versus 'click' events: Ideally we would fire an AJAX
+      // request for radios and checkboxes when they change, instead of every
+      // time they're clicked. Unfortunately, Internet Explorer doesn't fire
+      // change events correctly. For radio and checkbox elements, the change
+      // event fires only on blur (such as when the user clicks outside the form
+      // element). The workaround is to react on the click event instead of the
+      // change event. See http://www.quirksmode.org/dom/events/change.html#t04

Next to manual tests, we need a shorter version, retaining info, of that comment.

It's not clear to me what the first sentence tries to tell me (i.e., _why_ should we ideally fire an AJAX request on change instead of click). It looks like we can drop that entire sentence.

"Unfortunately" can be removed, IE can be shortened, and we don't need to explain the onBlur event.

Lastly, "See ..." should be "@see ..." on a separate line.

Powered by Dreditor.

I would however like effulgentsia's take on whether the abstraction he proposed would still be useful in other contexts.

If not needed to resolve this issue, then I don't think we should add #ajax['allow_default_action'] (or alternate name, see #39) as part of this issue.

However, something we don't have any use-cases in core for, but may have in contrib, is developers setting #ajax['event'] to something explicit, rather than using the defaults in ajax_pre_render_element(). It seems quite possible that in some of these cases, the developer wouldn't want the browser default action for the event prevented. So I think a separate issue for a #ajax['allow_default_action'] setting would make sense. I'll wait until this one is resolved before opening that.

If it's too late to add it for D7, I'm not overly concerned, because a contrib module can deal with it if needed. #36 was intended to remove per-type special casing within the JS code, because I think files like ajax.js and other generic behaviors should stay generic, and use Drupal.settings for customizations. If we can remove that special casing from the JS code without adding any new #ajax properties, great!

We should consider to use a property name closer to standards -- isn't this called "bubbles" normally?

Possibly irrelevant for now, as per above, but I'm not sure there's much of a standard here. There's "bubbles", "preventDefault", "stopPropagation", and probably other terms commonly in use. Returning false is the same as calling both event.preventDefault() and event.stopPropagation(), but I think it's the former that we care about more when we do it.

Also, it would be great to get merlinofchaos reviewing issues like this that change what event we're binding to, during the final days of D7 pre-release. Ctools, Views, and Panels are probably some of the biggest users of the AJAX API.

merlinofchaos won't be available for patch review until his son is at least a few days old. This isn't so much an AJAX thing as it is a straight JS event thing. We need a bunch of manual testing.

I dislike the idea of changing from 'change' to 'click', especially for radio, because it means triggering an AJAX request even when the radio doesn't change, and also, because of potential other browser quirks like this one from http://www.quirksmode.org/dom/events/click.html:

IE, Firefox and Opera allows users to check radios in the same group by using the arrow keys and the space bar once they've tabbed to one of the radios. In that case the click event fires, too, except in Opera 9.26.

Meanwhile, in http://api.jquery.com/change/, there is this claim:

As of jQuery 1.4 the change event now bubbles, and works identically to all other browsers, in Internet Explorer.

And apparently, based on the fact that this issue was considered fixed, at least for IE6/7/8, up until #38, it seems like jQuery does fix this properly. Perhaps it just fails to correct for IE9, but maybe plans to before IE9 is released. Needs more research.

Some time soon, I will roll up several outstanding AJAX event handling issues into one. When I do that, I will add a link to it here, and possibly mark this one a dupe. [EDIT: never mind. I realized I can't do what I was thinking of without breaking BC, so I'll save it for D8. We should proceed with each remaining issue independently].

Bleep bloop

any chance to get a working patch for D6?

Unfortunately it does not (I only applied the patch from #17 to ahah.js).
My test radios field with 'event' => 'click' does trigger the AJAX request but does not keep the selected option. With 'event' =>'change' the field needs to be clicked twice.

Edit: Funnily enough another radios field with "display: inline;" and the selected value being shifted into another variable seems to work fine with 'event' => 'click'. Very confusing...

1 year later, where do we stand?

Well that was easy, thanks for checking :)

There were some changes around JS for radio and checkboxes working on other issues. I guess it just fixed itself, a patch, newer jQuery? who knows :p

This patch was never ported to D7, here is a roll against D7.

Hm. How to reconcile that with #57/#58? @jucallme: do you have steps for how to replicate the bug?

Arrg, broken patch fixed.

@effulgentsia I have a basic ajax callback on a radio button group, when selecting them in IE they don't fire the callback as described in this thread, once this patch is applied they do.