fix(ngOptions): remove selected attribute from unselected options

When the select model changes, we add the "selected" attribute to the selected option, so that
screen readers know which option is selected.

Previously, we failed to remove the attribute from the selected / empty option when the model changed
to match a different option, or the unknown / empty option.

When using "track by", the behavior would also show when a user selected an option, and then the
model was changed, because track by watches the tracked expression, and calls the $render function
on change.

This fix reads the current select value, finds the matching option and removes the "selected"
attribute.

IE9 had to be special cased, as it will report option.hasAttribute('selected') === true even
if the option's property and attribute have been unset (even the dev tools show not selected attribute).

I've added a custom matcher that accounts for this behavior. In all other browsers, property and
attribute should always be in the same state. Since few people will use screen readers with IE9, I
hope this is a satisfactory solution to the problem.

Fixes angular#14892
Fixes angular#14419
Related angular#12731

Author: Narretz

src/ng/directive/ngOptions.js

@@ -448,10 +448,11 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
       var removeEmptyOption = function() {
         if (!providedEmptyOption) {
           emptyOption.remove();
+        } else {
+          emptyOption.removeAttr('selected');
         }
       };
 
-
       var renderUnknownOption = function() {
         selectElement.prepend(unknownOption);
         selectElement.val('?');
@@ -467,8 +468,13 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
       if (!multiple) {
 
         selectCtrl.writeValue = function writeNgOptionsValue(value) {
+          var selectedOption = options.selectValueMap[selectElement.val()];
           var option = options.getOptionFromViewValue(value);
 
+          // Make sure to remove the selected attribute from the previously selected option
+          // Otherwise, screen readers might get confused
+          if (selectedOption) selectedOption.element.removeAttribute('selected');
+
           if (option) {
             // Don't update the option when it is already selected.
             // For example, the browser will select the first option by default. In that case,
@@ -509,6 +515,7 @@ var ngOptionsDirective = ['$compile', '$document', '$parse', function($compile,
 
         // If we are using `track by` then we must watch the tracked value on the model
         // since ngModel only watches for object identity change
+        // FIXME: When a user selects an option, this watch will fire needlessly
         if (ngOptions.trackBy) {
           scope.$watch(
             function() { return ngOptions.getTrackByValue(ngModelCtrl.$viewValue); },

test/ng/directive/ngOptionsSpec.js

@@ -120,6 +120,55 @@ describe('ngOptions', function() {
             return { pass: errors.length === 0, message: message };
           }
         };
+      },
+      toBeMarkedAsSelected: function() {
+        // Selected is special because the element property and attribute reflect each other's state.
+        // IE9 will wrongly report hasAttribute('selected') === true when the property is
+        // undefined or null, and the dev tools show that no attribute is set
+        return {
+          compare: function(actual) {
+            var errors = [];
+            if (actual.selected === null || typeof actual.selected === 'undefined' || actual.selected === false) {
+              errors.push('Expected option property "selected" to be truthy');
+            }
+
+            if (msie !== 9 && actual.hasAttribute('selected') === false) {
+              errors.push('Expected option to have attribute "selected"');
+            }
+
+            var message = function() {
+              return errors.join('\n');
+            };
+
+            var result = {
+              pass: errors.length === 0,
+              message: message()
+            };
+
+            return result;
+          },
+          negativeCompare: function(actual) {
+            var errors = [];
+            if (actual.selected) {
+              errors.push('Expected option property "selected" to be falsy');
+            }
+
+            if (msie !== 9 && actual.hasAttribute('selected')) {
+              errors.push('Expected option not to have attribute "selected"');
+            }
+
+            var message = function() {
+              return errors.join('\n');
+            };
+
+            var result = {
+              pass: errors.length === 0,
+              message: message()
+            };
+
+            return result;
+          }
+        };
       }
     });
   });
@@ -744,6 +793,41 @@ describe('ngOptions', function() {
   });
 
 
+  it('should remove the "selected" attribute from the previous option when the model changes', function() {
+    scope.values = [{id: 10, label: 'ten'}, {id:20, label: 'twenty'}];
+
+    createSelect({
+      'ng-model': 'selected',
+      'ng-options': 'item.label for item in values'
+    }, true);
+
+    var options = element.find('option');
+    expect(options[0]).toBeMarkedAsSelected();
+    expect(options[1]).not.toBeMarkedAsSelected();
+    expect(options[2]).not.toBeMarkedAsSelected();
+
+    scope.selected = scope.values[0];
+    scope.$digest();
+
+    expect(options[0]).not.toBeMarkedAsSelected();
+    expect(options[1]).toBeMarkedAsSelected();
+    expect(options[2]).not.toBeMarkedAsSelected();
+
+    scope.selected = scope.values[1];
+    scope.$digest();
+
+    expect(options[0]).not.toBeMarkedAsSelected();
+    expect(options[1]).not.toBeMarkedAsSelected();
+    expect(options[2]).toBeMarkedAsSelected();
+
+    scope.selected = 'no match';
+    scope.$digest();
+
+    expect(options[0]).toBeMarkedAsSelected();
+    expect(options[1]).not.toBeMarkedAsSelected();
+    expect(options[2]).not.toBeMarkedAsSelected();
+  });
+
   describe('disableWhen expression', function() {
 
     describe('on single select', function() {
@@ -1395,6 +1479,26 @@ describe('ngOptions', function() {
         });
       }).not.toThrow();
     });
+
+    it('should remove the "selected" attribute when the model changes', function() {
+      createSelect({
+        'ng-model': 'selected',
+        'ng-options': 'item.label for item in arr track by item.id'
+      });
+
+      var options = element.find('option');
+      browserTrigger(options[2], 'click');
+
+      expect(scope.selected).toEqual(scope.arr[1]);
+
+      scope.selected = {};
+      scope.$digest();
+
+      expect(options[0]).toBeMarkedAsSelected();
+      expect(options[2]).not.toBeMarkedAsSelected();
+      expect(options[2]).not.toBeMarkedAsSelected();
+    });
+
   });