From d851e8f3cc3104a1b4b5ca103931b5baf0b6f48a Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Mon, 4 Jul 2016 14:11:55 +0200 Subject: [PATCH] fix(select): don't register options when select has no ngModel When option elements use ngValue, value or interpolated text to set the option value, i.e. when the parent select doesn't have an ngModel, there is no necessity in registering the options with the select controller. The registration was usually not a problem, as the ngModelController is set to a noop controller, so that all further interactions are aborted ($render etc) However, since f02b707 ngValue sets a hashed value inside the option value (to support arbitrary value types). This can cause issues with tests that expect unhashed values. The issue was found in angular-material, which uses select + ngValue to populate mdSelect. POSSIBLE BREAKING CHANGE: Option elements will no longer set their value attribute from their text value when their select element has no ngModel associated. Setting the value is only needed for the select directive to match model values and options. If no ngModel is present, the select directive doesn't need it. This should not affect many applications as the behavior was undocumented and not part of a public API. It also has no effect on the usual HTML5 behavior that sets the select value to the option text if the option does not provide a value attribute. --- src/ng/directive/select.js | 11 ++++++++--- test/ng/directive/selectSpec.js | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index b583546d653f..59b505323d91 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -519,11 +519,16 @@ var selectDirective = function() { function selectPreLink(scope, element, attr, ctrls) { - // if ngModel is not defined, we don't need to do anything + var selectCtrl = ctrls[0]; var ngModelCtrl = ctrls[1]; - if (!ngModelCtrl) return; - var selectCtrl = ctrls[0]; + // if ngModel is not defined, we don't need to do anything but set the registerOption + // function to noop, so options don't get added internally + if (!ngModelCtrl) { + selectCtrl.registerOption = noop; + return; + } + selectCtrl.ngModelCtrl = ngModelCtrl; diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 9027aa657326..17f1a6b26f74 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -136,6 +136,30 @@ describe('select', function() { }); + it('should not add options to the select if ngModel is not present', inject(function($rootScope) { + var scope = $rootScope; + scope.d = 'd'; + scope.e = 'e'; + scope.f = 'f'; + + compile(''); + + var selectCtrl = element.controller('select'); + + expect(selectCtrl.hasOption('a')).toBe(false); + expect(selectCtrl.hasOption('b')).toBe(false); + expect(selectCtrl.hasOption('c')).toBe(false); + expect(selectCtrl.hasOption('d')).toBe(false); + expect(selectCtrl.hasOption('e')).toBe(false); + expect(selectCtrl.hasOption('f')).toBe(false); + })); describe('select-one', function() {