Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 34fe2ee

Browse files
committedNov 26, 2014
fix(select): use strict compare when removing option from ctrl
Otherwise, if the removed option was the empty option (value ''), and the currently selected option had a value of 0, the select would think that the currently selected option had been removed, causing the unknown option to be added again. Fixes angular#9714 Fixes angular#10115 Closes angular#10203
1 parent 8e2aa27 commit 34fe2ee

File tree

2 files changed

+23
-22
lines changed

2 files changed

+23
-22
lines changed
 

‎src/ng/directive/select.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -209,10 +209,10 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
209209
};
210210

211211

212-
self.removeOption = function(value, anySelected) {
212+
self.removeOption = function(value) {
213213
if (this.hasOption(value)) {
214214
delete optionsMap[value];
215-
if (ngModelCtrl.$viewValue == value && !anySelected) {
215+
if (ngModelCtrl.$viewValue === value) {
216216
this.renderUnknownOption(value);
217217
}
218218
}
@@ -683,7 +683,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
683683
if (count > 0) {
684684
selectCtrl.addOption(label);
685685
} else if (count < 0) {
686-
selectCtrl.removeOption(label, anySelected);
686+
selectCtrl.removeOption(label);
687687
}
688688
});
689689
}

‎test/ng/directive/selectSpec.js

+20-19
Original file line numberDiff line numberDiff line change
@@ -233,30 +233,31 @@ describe('select', function() {
233233
expect(scope.robot).toBe('');
234234
});
235235

236-
it('should not add the empty string when an option is selected and options are set in a timeout', inject(function($timeout) {
236+
it('should not be set when an option is selected and options are set asynchronously',
237+
inject(function($timeout) {
238+
compile('<select ng-model="model" ng-options="opt.id as opt.label for opt in options">' +
239+
'</select>');
237240

238-
compile('<select ng-model="simpleModel" ng-options="opt.id as opt.label for opt in simpleOpts">' +
239-
'</select>');
240-
241-
scope.$apply(function() {
242-
scope.simpleModel = 0;
243-
});
241+
scope.$apply(function() {
242+
scope.model = 0;
243+
});
244244

245-
$timeout(function() {
246-
scope.simpleOpts = [
247-
{id: 0, label: 'x'},
248-
{id: 1, label: 'y'}
249-
];
250-
}, 0);
245+
$timeout(function() {
246+
scope.options = [
247+
{id: 0, label: 'x'},
248+
{id: 1, label: 'y'}
249+
];
250+
}, 0);
251251

252-
$timeout.flush();
252+
$timeout.flush();
253253

254-
var options = element.find('option');
254+
var options = element.find('option');
255255

256-
expect(options.length).toEqual(2);
257-
expect(options.eq(0)).toEqualOption('0', 'x');
258-
expect(options.eq(1)).toEqualOption('1', 'y');
259-
}));
256+
expect(options.length).toEqual(2);
257+
expect(options.eq(0)).toEqualOption('0', 'x');
258+
expect(options.eq(1)).toEqualOption('1', 'y');
259+
})
260+
);
260261

261262
describe('interactions with repeated options', function() {
262263

1 commit comments

Comments
 (1)

hamfastgamgee commented on Nov 26, 2014

@hamfastgamgee

@Narretz Like I mentioned in angular#10115, addOption has the same == check. Does that need to be addressed, too? (I don't know what the consequences are, if indeed there are any. I'm just posing the question. :) )

Please sign in to comment.