Skip to content

Commit 4ddfa05

Browse files
author
Jeff Balboni
committed
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
1 parent 319dd1a commit 4ddfa05

File tree

2 files changed

+24
-3
lines changed

2 files changed

+24
-3
lines changed

src/ng/directive/select.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
322322
// We try to reuse these if possible
323323
// - optionGroupsCache[0] is the options with no option group
324324
// - optionGroupsCache[?][0] is the parent: either the SELECT or OPTGROUP element
325-
optionGroupsCache = [[{element: selectElement, label:''}]];
325+
optionGroupsCache = [[{element: selectElement, label:''}]],
326+
changeEventFired = false;
326327

327328
if (nullOption) {
328329
// compile the element since there might be bindings in it
@@ -341,6 +342,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
341342
selectElement.empty();
342343

343344
selectElement.on('change', function() {
345+
changeEventFired = true;
344346
scope.$apply(function() {
345347
var optionGroup,
346348
collection = valuesFn(scope) || [],
@@ -409,8 +411,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
409411
optionGroupName,
410412
optionGroup,
411413
option,
412-
existingParent, existingOptions, existingOption,
413414
modelValue = ctrl.$modelValue,
415+
existingParent, existingOptions, existingOption,
414416
values = valuesFn(scope) || [],
415417
keys = keyName ? sortedKeys(values) : values,
416418
key,
@@ -529,7 +531,10 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
529531
lastElement.val(existingOption.id = option.id);
530532
}
531533
// lastElement.prop('selected') provided by jQuery has side-effects
532-
if (lastElement[0].selected !== option.selected) {
534+
// FF will update selected property on DOM elements when hovering,
535+
// so we shouldn't check those unless a change event has happened
536+
if ((changeEventFired && lastElement[0].selected !== option.selected) ||
537+
existingOption.selected !== option.selected) {
533538
lastElement.prop('selected', (existingOption.selected = option.selected));
534539
}
535540
} else {
@@ -573,6 +578,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
573578
while(optionGroupsCache.length > groupIndex) {
574579
optionGroupsCache.pop()[0].element.remove();
575580
}
581+
changeEventFired = false;
576582
}
577583
}
578584
}

test/ng/directive/selectSpec.js

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

736+
it('should ignore option object selected changes', function() {
737+
createSingleSelect();
738+
739+
scope.$apply(function() {
740+
scope.values = [{name: 'A'}, {name: 'B'}, {name: 'C'}];
741+
scope.selected = scope.values[0];
742+
});
743+
744+
var options = element.find('option');
745+
var optionToSelect = options.eq(1);
746+
optionToSelect.prop('selected', true);
747+
scope.$digest();
748+
expect(optionToSelect.prop('selected')).toBe(true);
749+
});
750+
736751
describe('binding', function() {
737752

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

0 commit comments

Comments
 (0)