fix(select): avoid checking option element selected properties in render

Jeff Balboni · caitp · commit dc149de9364c · 2014-03-20T17:27:02.000-04:00
In Firefox, hovering over an option in an open select menu updates the selected property of option elements. This means that when a render is triggered by the digest cycle, and the list of options is being rendered, the selected properties are reset to the values from the model and the option hovered over changes. This fix changes the code to only use DOM elements' selected properties in a comparison when a change event has been fired. Otherwise, the internal new and existing option arrays are used. Closes angular#2448 Closes angular#5994 Closes angular#6769
diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js
@@ -394,6 +394,12 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
                   value = valueFn(scope, locals);
                 }
               }
+              // Update the null option's selected property here so $render cleans it up correctly
+              if (optionGroupsCache[0].length > 1) {
+                if (optionGroupsCache[0][1].id !== key) {
+                  optionGroupsCache[0][1].selected = false;
+                }
+              }
             }
             ctrl.$setViewValue(value);
           });
@@ -531,7 +537,7 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
                   lastElement.val(existingOption.id = option.id);
                 }
                 // lastElement.prop('selected') provided by jQuery has side-effects
-                if (lastElement[0].selected !== option.selected) {
+                if (existingOption.selected !== option.selected) {
                   lastElement.prop('selected', (existingOption.selected = option.selected));
                 }
               } else {
diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js
@@ -733,6 +733,27 @@ describe('select', function() {
       expect(sortedHtml(options[2])).toEqual('<option value="1">3</option>');
     });
 
+    it('should not update selected property of an option element on digest with no change event',
+        function() {
+      createSingleSelect();
+
+      scope.$apply(function() {
+        scope.values = [{name: 'A'}, {name: 'B'}, {name: 'C'}];
+        scope.selected = scope.values[0];
+      });
+
+      var options = element.find('option');
+      var optionToSelect = options.eq(1);
+
+      expect(optionToSelect.text()).toBe('B');
+
+      optionToSelect.prop('selected', true);
+      scope.$digest();
+
+      expect(optionToSelect.prop('selected')).toBe(true);
+      expect(scope.selected).toBe(scope.values[0]);
+    });
+
     describe('binding', function() {
 
       it('should bind to scope value', function() {

Original file line number	Diff line number	Diff line change
`@@ -394,6 +394,12 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {`
`394`	`394`	`value = valueFn(scope, locals);`
`395`	`395`	`}`
`396`	`396`	`}`
	`397`	`+ // Update the null option's selected property here so $render cleans it up correctly`
	`398`	`+ if (optionGroupsCache[0].length > 1) {`
	`399`	`+ if (optionGroupsCache[0][1].id !== key) {`
	`400`	`+ optionGroupsCache[0][1].selected = false;`
	`401`	`+ }`
	`402`	`+ }`
`397`	`403`	`}`
`398`	`404`	`ctrl.$setViewValue(value);`
`399`	`405`	`});`
`@@ -531,7 +537,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {`
`531`	`537`	`lastElement.val(existingOption.id = option.id);`
`532`	`538`	`}`
`533`	`539`	`// lastElement.prop('selected') provided by jQuery has side-effects`
`534`		`- if (lastElement[0].selected !== option.selected) {`
	`540`	`+ if (existingOption.selected !== option.selected) {`
`535`	`541`	`lastElement.prop('selected', (existingOption.selected = option.selected));`
`536`	`542`	`}`
`537`	`543`	`} else {`