fixup! refactor selected attr / prop setting

Author: Narretz

src/ng/directive/select.js

@@ -4,9 +4,16 @@
 
 var noopNgModelController = { $setViewValue: noop, $render: noop };
 
-function setOptionAsSelected(optionEl) {
-  optionEl.prop('selected', true); // needed for IE
-  optionEl.attr('selected', true);
+function setOptionSelectedStatus(optionEl, value) {
+  optionEl.prop('selected', value); // needed for IE
+  if (value) {
+    optionEl.attr('selected', value);
+  } else {
+    // Technically, setting the property to null / false is enough to unselect the option
+    // However, screenreaders might react to the selected attribute instead, see
+    // https://github.com/angular/angular.js/issues/14419
+    optionEl.removeAttr('selected');
+  }
 }
 
 /**
@@ -49,14 +56,14 @@ var SelectController =
     var unknownVal = self.generateUnknownOptionValue(val);
     self.unknownOption.val(unknownVal);
     $element.prepend(self.unknownOption);
-    setOptionAsSelected(self.unknownOption);
+    setOptionSelectedStatus(self.unknownOption, true);
     $element.val(unknownVal);
   };
 
   self.updateUnknownOption = function(val) {
     var unknownVal = self.generateUnknownOptionValue(val);
     self.unknownOption.val(unknownVal);
-    setOptionAsSelected(self.unknownOption);
+    setOptionSelectedStatus(self.unknownOption, true);
     $element.val(unknownVal);
   };
 
@@ -71,7 +78,7 @@ var SelectController =
   self.selectEmptyOption = function() {
     if (self.emptyOption) {
       $element.val('');
-      setOptionAsSelected(self.emptyOption);
+      setOptionSelectedStatus(self.emptyOption, true);
     }
   };
 
@@ -107,7 +114,7 @@ var SelectController =
     // Make sure to remove the selected attribute from the previously selected option
     // Otherwise, screen readers might get confused
     var currentlySelectedOption = $element[0].options[$element[0].selectedIndex];
-    if (currentlySelectedOption) currentlySelectedOption.removeAttribute('selected');
+    if (currentlySelectedOption) setOptionSelectedStatus(jqLite(currentlySelectedOption), false);
 
     if (self.hasOption(value)) {
       self.removeUnknownOption();
@@ -117,7 +124,7 @@ var SelectController =
 
       // Set selected attribute and property on selected option for screen readers
       var selectedOption = $element[0].options[$element[0].selectedIndex];
-      setOptionAsSelected(jqLite(selectedOption));
+      setOptionSelectedStatus(jqLite(selectedOption), true);
     } else {
       if (value == null && self.emptyOption) {
         self.removeUnknownOption();
@@ -617,12 +624,8 @@ var selectDirective = function() {
             // So we only modify the selected property if neccessary.
             // Note: this behavior cannot be replicated via unit tests because it only shows in the
             // actual user interface.
-            if ((shouldBeSelected && !currentlySelected)) {
-              setOptionAsSelected(jqLite(option));
-            } else if ((!shouldBeSelected && currentlySelected)) {
-              option.selected  = shouldBeSelected;
-              // Remove attribute to not confuse screen readers
-              option.removeAttribute('selected');
+            if (shouldBeSelected !== currentlySelected) {
+              setOptionSelectedStatus(jqLite(option), shouldBeSelected);
             }
 
           });