submitting radio button state does not work because removeAttr on radio buttons sets "checked" property of all related radio buttons to false #3482

Closed
shyblower opened this Issue Jan 24, 2012 · 6 comments

Projects

None yet

5 participants

@shyblower

This happens in jquery.mobile.forms.checkboxradio.js in line 90:
self._getInputSet().not( input ).removeAttr( "checked" );
after the correct radio button has been checked in line 77 using:
input.attr( "checked", inputtype === "radio" && true || !input.attr( "checked" ) );
After calling this "removeAttr" on only the unselected radio buttons the "checked" property (see prop vs. attr in the jquery docs) of the DOM objects of ALL radio buttons in the same set (carrying the same name attribute) will be set to false (the attribute value of the "checked" attribute in the HTML stays "checked") thus preventing radio button form elements from being transmitted on a submit (form.serialize() recognizes the state of all radio button inputs as false and thus does not serialize it).
Doing the removeAttr (line 90) before setting the "checked" attribute (line 77) resolves the issue.
Tested on Firefox 9 and Chromium 17.0.963.3 using jquery-1.7.1 with current HEAD of jquery.mobile.

cheers,
Tom

@agcolom
Member
agcolom commented Feb 20, 2012

Yes, I can confirm. I have 2 fiddles to illustrate:
http://jsfiddle.net/agcolom/9t4e6/51/ (uses latest)
http://jsfiddle.net/agcolom/9t4e6/50/ (uses 1.0.1)

Anne

@rbdcti
Contributor
rbdcti commented Feb 26, 2012

+1 on this issue. would think this one would need to be resolved before 1.1 is released, given that it breaks being able to get the value of a radio button widget (e.g. $(":input[name='radio-choice-1']:checked") is empty, even when an element is selected)

@rbdcti
Contributor
rbdcti commented Feb 26, 2012

I can also confirm that the fix suggested by @tscheibl does work. However I don't feel comfortable submitting a pull request because I'm not sure of the side effects involved in it (I've patched my own 1.1 repo for now...)

@rbdcti
Contributor
rbdcti commented Feb 26, 2012

Oops, sorry. I made a pull request from the second one. Had issues with the integrated editor and whitespace on the first so I deleted my fork and tried again.

@jakeboone02
Contributor

This is fixed in the latest -- see #3670 as @kihlstrom noted. Tested with the jsfiddle @agcolom mentioned above: http://jsfiddle.net/agcolom/9t4e6/51/

@agcolom
Member
agcolom commented Mar 1, 2012

@jakeboone02 Thanks a lot for checking this.. Yes, definitely fixed... @toddparker @gseguin @Wilto we can close. Thanks, Anne

@toddparker toddparker closed this Mar 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment