Skip to content

Commit 0fa8410

Browse files
committed
Bug where one radio button in group required
The HTML spec does not seemd to clearly state what "required" does when only some of a radio button set are required. This answer on StackOverflow states that only one of the set is required: http://stackoverflow.com/a/8287947/1009332 Per the comments added in the fix: We cannot just compare allInputs for radio buttons, as the spec for required radio buttons states that only one of a radio button set needs to be required. If you have 2 radio buttons, and one is checked, but it's not required, then we get the other required radio button and see that there are no other radio buttons checked for the "required" ones. Thus, if the input control is a radio button, then filter for all radio buttons with the same name on the form. This will get all radio buttons of a given name that are checked and see that at least one is checked. Don't count unchecked required radio if other radio with same name is checked. But do count checked non-required radio of the same name. Note, each radio button name that is required is checked only once.
1 parent 15515b2 commit 0fa8410

File tree

2 files changed

+61
-15
lines changed

2 files changed

+61
-15
lines changed

src/rails.js

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -309,24 +309,45 @@
309309

310310
// Helper function which checks for blank inputs in a form that match the specified CSS selector
311311
blankInputs: function(form, specifiedSelector, nonBlank) {
312-
var inputs = $(), input, valueToCheck,
313-
selector = specifiedSelector || 'input,textarea',
314-
allInputs = form.find(selector);
315-
316-
allInputs.each(function() {
312+
var foundInputs = $(),
313+
input,
314+
valueToCheck,
315+
radiosForNameWithNoneSelected,
316+
radioName,
317+
selector = specifiedSelector || 'input,textarea',
318+
requiredInputs = form.find(selector),
319+
checkedRadioButtonNames = {};
320+
321+
requiredInputs.each(function() {
317322
input = $(this);
318-
valueToCheck = input.is('input[type=checkbox],input[type=radio]') ? input.is(':checked') : !!input.val();
319-
if (valueToCheck === nonBlank) {
323+
if (input.is('input[type=radio]')) {
320324

321-
// Don't count unchecked required radio if other radio with same name is checked
322-
if (input.is('input[type=radio]') && allInputs.filter('input[type=radio]:checked[name="' + input.attr('name') + '"]').length) {
323-
return true; // Skip to next input
324-
}
325+
// Don't count unchecked required radio as blank if other radio with same name is checked,
326+
// regardless of whether same-name radio input has required attribute or not. The spec
327+
// states https://www.w3.org/TR/html5/forms.html#the-required-attribute
328+
radioName = input.attr('name');
329+
330+
// Skip if we've already seen the radio with this name.
331+
if (!checkedRadioButtonNames[radioName]) {
332+
333+
// If none checked
334+
if (form.find('input[type=radio]:checked[name="' + radioName + '"]').length === 0) {
335+
radiosForNameWithNoneSelected = form.find(
336+
'input[type=radio][name="' + radioName + '"]');
337+
foundInputs = foundInputs.add(radiosForNameWithNoneSelected);
338+
}
325339

326-
inputs = inputs.add(input);
340+
// We only need to check each name once.
341+
checkedRadioButtonNames[radioName] = radioName;
342+
}
343+
} else {
344+
valueToCheck = input.is('input[type=checkbox],input[type=radio]') ? input.is(':checked') : !!input.val();
345+
if (valueToCheck === nonBlank) {
346+
foundInputs = foundInputs.add(input);
347+
}
327348
}
328349
});
329-
return inputs.length ? inputs : false;
350+
return foundInputs.length ? foundInputs : false;
330351
},
331352

332353
// Helper function which checks for non-blank inputs in a form that match the specified CSS selector

test/public/test/call-remote-callbacks.js

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,8 @@ asyncTest('unchecked required checkbox should abort form submission', 1, functio
241241

242242
asyncTest('unchecked required radio should abort form submission', 1, function() {
243243
var form = $('form[data-remote]')
244-
.append($('<input type="radio" name="yes_no" required="required" value=1>'))
245-
.append($('<input type="radio" name="yes_no" required="required" value=2>'))
244+
.append($('<input type="radio" name="yes_no_none" required="required" value=1>'))
245+
.append($('<input type="radio" name="yes_no_none" required="required" value=2>'))
246246
.removeAttr('data-remote')
247247
.bind('ujs:everythingStopped', function() {
248248
ok(true, 'ujs:everythingStopped should run');
@@ -275,6 +275,31 @@ asyncTest('required radio should only require one to be checked', 1, function()
275275
}, 13);
276276
});
277277

278+
asyncTest('required radio should only require one to be checked if not all radios are required', 1, function() {
279+
$(document).bind('iframe:loading', function() {
280+
ok(true, 'form should get submitted');
281+
});
282+
283+
var form = $('form[data-remote]')
284+
// Check the radio that is not required
285+
.append($('<input type="radio" name="yes_no_maybe" value=1 >'))
286+
// Check the radio that is not required
287+
.append($('<input type="radio" name="yes_no_maybe" value=2 id="checkme">'))
288+
// Only one needs to be required
289+
.append($('<input type="radio" name="yes_no_maybe" required="required" value=3>'))
290+
.removeAttr('data-remote')
291+
.bind('ujs:everythingStopped', function() {
292+
ok(false, 'ujs:everythingStopped should not run');
293+
})
294+
.find('#checkme').prop('checked', true)
295+
.end()
296+
.trigger('submit');
297+
298+
setTimeout(function() {
299+
start();
300+
}, 13);
301+
});
302+
278303
function skipIt() {
279304
// This test cannot work due to the security feature in browsers which makes the value
280305
// attribute of file input fields readonly, so it cannot be set with default value.

0 commit comments

Comments
 (0)