Skip to content

Commit 054f9c8

Browse files
Jeff Balbonicaitp
Jeff Balboni
authored andcommitted
fix(select): avoid checking option element selected properties in render
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
1 parent 37bc5ef commit 054f9c8

File tree

2 files changed

+28
-1
lines changed

2 files changed

+28
-1
lines changed

src/ng/directive/select.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,12 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
394394
value = valueFn(scope, locals);
395395
}
396396
}
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+
}
397403
}
398404
ctrl.$setViewValue(value);
399405
});
@@ -531,7 +537,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
531537
lastElement.val(existingOption.id = option.id);
532538
}
533539
// lastElement.prop('selected') provided by jQuery has side-effects
534-
if (lastElement[0].selected !== option.selected) {
540+
if (existingOption.selected !== option.selected) {
535541
lastElement.prop('selected', (existingOption.selected = option.selected));
536542
}
537543
} else {

test/ng/directive/selectSpec.js

+21
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,27 @@ describe('select', function() {
733733
expect(sortedHtml(options[2])).toEqual('<option value="1">3</option>');
734734
});
735735

736+
it('should not update selected property of an option element on digest with no change event',
737+
function() {
738+
createSingleSelect();
739+
740+
scope.$apply(function() {
741+
scope.values = [{name: 'A'}, {name: 'B'}, {name: 'C'}];
742+
scope.selected = scope.values[0];
743+
});
744+
745+
var options = element.find('option');
746+
var optionToSelect = options.eq(1);
747+
748+
expect(optionToSelect.text()).toBe('B');
749+
750+
optionToSelect.prop('selected', true);
751+
scope.$digest();
752+
753+
expect(optionToSelect.prop('selected')).toBe(true);
754+
expect(scope.selected).toBe(scope.values[0]);
755+
});
756+
736757
describe('binding', function() {
737758

738759
it('should bind to scope value', function() {

0 commit comments

Comments
 (0)