From df0bc97d7a9fcd46159714d6c3287569e730146f Mon Sep 17 00:00:00 2001
From: Martin Staffa <mjstaffa@gmail.com>
Date: Mon, 11 Jul 2016 15:39:19 +0200
Subject: [PATCH] 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 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.

Fixes #14892
Fixes #14419
Related #12731
---
 src/ng/directive/ngOptions.js      |  6 ++++
 test/ng/directive/ngOptionsSpec.js | 53 ++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js
index 10d970097f02..684b2cfb6cd8 100644
--- a/src/ng/directive/ngOptions.js
+++ b/src/ng/directive/ngOptions.js
@@ -467,8 +467,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 +514,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); },
diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js
index 8fb6a2a9fa3f..73f9813c6ada 100644
--- a/test/ng/directive/ngOptionsSpec.js
+++ b/test/ng/directive/ngOptionsSpec.js
@@ -743,6 +743,37 @@ describe('ngOptions', function() {
     expect(options.eq(3)).toEqualOption('c');
   });
 
+  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 resetButton = angular.element('<input type="reset" />');
+    formElement.append(resetButton);
+
+    scope.selected = scope.values[0];
+    scope.$digest();
+
+    var options = element.find('option');
+    expect(options[1].getAttribute('selected')).toBe('selected');
+
+    scope.selected = scope.values[1];
+    scope.$digest();
+
+    expect(options[1].getAttribute('selected')).toBe(null);
+    expect(options[2].getAttribute('selected')).toBe('selected');
+
+    scope.selected = 'no match';
+    scope.$digest();
+
+    expect(options[0].selected).toBe(true);
+    expect(options[0].getAttribute('selected')).toBe('selected');
+    expect(options[1].getAttribute('selected')).toBe(null);
+    expect(options[2].getAttribute('selected')).toBe(null);
+  });
 
   describe('disableWhen expression', function() {
 
@@ -1395,6 +1426,28 @@ 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();
+
+      options[2].selected = false;
+      expect(options[0].selected).toBe(true);
+      expect(options[0].getAttribute('selected')).toBe('selected');
+      expect(options[2].selected).toBe(false);
+      expect(options[2].getAttribute('selected')).toBe(null);
+    });
+
   });