Merge pull request #457 from justin808/justin808-fix-radio-buttons-re…

…quired Bug where one radio button in group required
rails · Feb 23, 2016 · b0bd907 · b0bd907
2 parents 0c6efb6 + 0fa8410
commit b0bd907
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 15 deletions.
diff --git a/src/rails.js b/src/rails.js
@@ -309,24 +309,45 @@
 
     // Helper function which checks for blank inputs in a form that match the specified CSS selector
     blankInputs: function(form, specifiedSelector, nonBlank) {
-      var inputs = $(), input, valueToCheck,
-          selector = specifiedSelector || 'input,textarea',
-          allInputs = form.find(selector);
-
-      allInputs.each(function() {
+      var foundInputs = $(),
+        input,
+        valueToCheck,
+        radiosForNameWithNoneSelected,
+        radioName,
+        selector = specifiedSelector || 'input,textarea',
+        requiredInputs = form.find(selector),
+        checkedRadioButtonNames = {};
+
+      requiredInputs.each(function() {
         input = $(this);
-        valueToCheck = input.is('input[type=checkbox],input[type=radio]') ? input.is(':checked') : !!input.val();
-        if (valueToCheck === nonBlank) {
+        if (input.is('input[type=radio]')) {
 
-          // Don't count unchecked required radio if other radio with same name is checked
-          if (input.is('input[type=radio]') && allInputs.filter('input[type=radio]:checked[name="' + input.attr('name') + '"]').length) {
-            return true; // Skip to next input
-          }
+          // Don't count unchecked required radio as blank if other radio with same name is checked,
+          // regardless of whether same-name radio input has required attribute or not. The spec
+          // states https://www.w3.org/TR/html5/forms.html#the-required-attribute
+          radioName = input.attr('name');
+
+          // Skip if we've already seen the radio with this name.
+          if (!checkedRadioButtonNames[radioName]) {
+
+            // If none checked
+            if (form.find('input[type=radio]:checked[name="' + radioName + '"]').length === 0) {
+              radiosForNameWithNoneSelected = form.find(
+                'input[type=radio][name="' + radioName + '"]');
+              foundInputs = foundInputs.add(radiosForNameWithNoneSelected);
+            }
 
-          inputs = inputs.add(input);
+            // We only need to check each name once.
+            checkedRadioButtonNames[radioName] = radioName;
+          }
+        } else {
+          valueToCheck = input.is('input[type=checkbox],input[type=radio]') ? input.is(':checked') : !!input.val();
+          if (valueToCheck === nonBlank) {
+            foundInputs = foundInputs.add(input);
+          }
         }
       });
-      return inputs.length ? inputs : false;
+      return foundInputs.length ? foundInputs : false;
     },
 
     // Helper function which checks for non-blank inputs in a form that match the specified CSS selector

diff --git a/test/public/test/call-remote-callbacks.js b/test/public/test/call-remote-callbacks.js
@@ -241,8 +241,8 @@ asyncTest('unchecked required checkbox should abort form submission', 1, functio
 
 asyncTest('unchecked required radio should abort form submission', 1, function() {
   var form = $('form[data-remote]')
-    .append($('<input type="radio" name="yes_no" required="required" value=1>'))
-    .append($('<input type="radio" name="yes_no" required="required" value=2>'))
+    .append($('<input type="radio" name="yes_no_none" required="required" value=1>'))
+    .append($('<input type="radio" name="yes_no_none" required="required" value=2>'))
     .removeAttr('data-remote')
     .bind('ujs:everythingStopped', function() {
       ok(true, 'ujs:everythingStopped should run');
@@ -275,6 +275,31 @@ asyncTest('required radio should only require one to be checked', 1, function()
   }, 13);
 });
 
+asyncTest('required radio should only require one to be checked if not all radios are required', 1, function() {
+  $(document).bind('iframe:loading', function() {
+    ok(true, 'form should get submitted');
+  });
+
+  var form = $('form[data-remote]')
+    // Check the radio that is not required
+    .append($('<input type="radio" name="yes_no_maybe" value=1 >'))
+    // Check the radio that is not required
+    .append($('<input type="radio" name="yes_no_maybe" value=2 id="checkme">'))
+    // Only one needs to be required
+    .append($('<input type="radio" name="yes_no_maybe" required="required" value=3>'))
+    .removeAttr('data-remote')
+    .bind('ujs:everythingStopped', function() {
+      ok(false, 'ujs:everythingStopped should not run');
+    })
+    .find('#checkme').prop('checked', true)
+    .end()
+    .trigger('submit');
+
+  setTimeout(function() {
+    start();
+  }, 13);
+});
+
 function skipIt() {
   // This test cannot work due to the security feature in browsers which makes the value
   // attribute of file input fields readonly, so it cannot be set with default value.