From 580dcdc73d7be2a5298e8d777855d3c45c8c6203 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 29 Sep 2015 10:50:59 +0100 Subject: [PATCH 1/3] fix(ngOptions): disable select directive when ngOptions is active --- src/ng/directive/ngOptions.js | 1 + src/ng/directive/select.js | 13 +++++++++---- test/ng/directive/ngOptionsSpec.js | 23 +++++++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/ng/directive/ngOptions.js b/src/ng/directive/ngOptions.js index 282dff3aedd2..561061b07020 100644 --- a/src/ng/directive/ngOptions.js +++ b/src/ng/directive/ngOptions.js @@ -395,6 +395,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { return { restrict: 'A', terminal: true, + controller: function() { }, require: ['select', 'ngModel'], link: function(scope, selectElement, attr, ctrls) { diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index df2bcee32437..2f9aca5db9fc 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -417,10 +417,15 @@ var optionDirective = ['$interpolate', function($interpolate) { // This is an optimization over using ^^ since we don't want to have to search // all the way to the root of the DOM for every single option element - var selectCtrlName = '$selectController', - parent = element.parent(), - selectCtrl = parent.data(selectCtrlName) || - parent.parent().data(selectCtrlName); // in case we are in optgroup + var selectElement = element.parent(); + if (nodeName_(selectElement) !== 'select') { + selectElement = selectElement.parent(); // in case we are in optgroup + } + + var selectCtrl = selectElement.data('$selectController'); + + // If there is an ngOptions directive then bail out + if (selectElement.data('$ngOptionsController')) return; function addOption(optionValue) { selectCtrl.addOption(optionValue, element); diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index bc50bcbef818..9ba299811264 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -104,6 +104,17 @@ describe('ngOptions', function() { }); }); + beforeEach(module(function($compileProvider) { + $compileProvider + .directive('compileContents', function($compile) { + return { + link: { pre: function(scope, element) { + $compile(element.contents())(scope); + }} + }; + }); + })); + beforeEach(inject(function($rootScope, _$compile_) { scope = $rootScope.$new(); //create a child scope because the root scope can't be $destroy-ed $compile = _$compile_; @@ -2119,6 +2130,18 @@ describe('ngOptions', function() { option = element.find('option').eq(0); expect(option.text()).toBe('A'); }); + + + it('should not throw when a directive compiles the blank option before ngOptions is linked', function() { + expect(function() { + createSelect({ + 'compile-contents': '', + 'name': 'select', + 'ng-model': 'value', + 'ng-options': 'item for item in items', + }, true); + }).not.toThrow(); + }); }); From b37bc816d716b448ff8cef173c9f076baa107281 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 29 Sep 2015 21:02:44 +0100 Subject: [PATCH 2/3] test(ngOptions): check that replace+templateUrl doesn't break --- test/ng/directive/ngOptionsSpec.js | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index 9ba299811264..0e808982d4d0 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -112,7 +112,18 @@ describe('ngOptions', function() { $compile(element.contents())(scope); }} }; - }); + }) + .directive('customSelect', function() { + return { + restrict: 'E', + replace: true, + scope: { ngModel: '=', options: '='}, + templateUrl: 'select_template.html', + link: function(scope) { + scope.selectable_options = scope.options; + } + } + }); })); beforeEach(inject(function($rootScope, _$compile_) { @@ -2142,6 +2153,19 @@ describe('ngOptions', function() { }, true); }).not.toThrow(); }); + + it('should not throw with a directive that replaces', inject(function($templateCache, $httpBackend) { + $templateCache.put('select_template.html', ''); + + scope.options = ['a', 'b', 'c', 'd']; + + expect(function() { + var element = $compile('')(scope); + scope.$digest(); + dealoc(element); + }).not.toThrow(); + + })); }); From a3dfabe62d28f2f12ecd0e3d7747fdd715f844c0 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Tue, 29 Sep 2015 21:24:55 +0100 Subject: [PATCH 3/3] squash me --- test/ng/directive/ngOptionsSpec.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/ng/directive/ngOptionsSpec.js b/test/ng/directive/ngOptionsSpec.js index 0e808982d4d0..4a5361352381 100644 --- a/test/ng/directive/ngOptionsSpec.js +++ b/test/ng/directive/ngOptionsSpec.js @@ -108,9 +108,11 @@ describe('ngOptions', function() { $compileProvider .directive('compileContents', function($compile) { return { - link: { pre: function(scope, element) { - $compile(element.contents())(scope); - }} + link: { + pre: function(scope, element) { + $compile(element.contents())(scope); + } + } }; }) .directive('customSelect', function() { @@ -2145,7 +2147,7 @@ describe('ngOptions', function() { it('should not throw when a directive compiles the blank option before ngOptions is linked', function() { expect(function() { - createSelect({ + createSelect({ 'compile-contents': '', 'name': 'select', 'ng-model': 'value',