Skip to content

Commit

Permalink
fix(select): avoid checking option element selected properties in render
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Jeff Balboni committed Jan 26, 2014
1 parent 319dd1a commit 0a284b1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
// We try to reuse these if possible
// - optionGroupsCache[0] is the options with no option group
// - optionGroupsCache[?][0] is the parent: either the SELECT or OPTGROUP element
optionGroupsCache = [[{element: selectElement, label:''}]];
optionGroupsCache = [[{element: selectElement, label:''}]],
changeEventFired = false;

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

selectElement.on('change', function() {
changeEventFired = true;
scope.$apply(function() {
var optionGroup,
collection = valuesFn(scope) || [],
Expand Down Expand Up @@ -529,7 +531,10 @@ 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) {
// FF will update selected property on DOM elements when hovering,
// so we shouldn't check those unless a change event has happened
if ((changeEventFired && lastElement[0].selected !== option.selected) ||
existingOption.selected !== option.selected) {
lastElement.prop('selected', (existingOption.selected = option.selected));
}
} else {
Expand Down Expand Up @@ -573,6 +578,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
while(optionGroupsCache.length > groupIndex) {
optionGroupsCache.pop()[0].element.remove();
}
changeEventFired = false;
}
}
}
Expand Down
15 changes: 15 additions & 0 deletions test/ng/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,21 @@ describe('select', function() {
expect(sortedHtml(options[2])).toEqual('<option value="1">3</option>');
});

it('should ignore option object selected changes', 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);
optionToSelect.prop('selected', true);
scope.$digest();
expect(optionToSelect.prop('selected')).toBe(true);
});

describe('binding', function() {

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

0 comments on commit 0a284b1

Please sign in to comment.