Skip to content

Commit

Permalink
fix(select): don't support binding to select[multiple]
Browse files Browse the repository at this point in the history
changing the type of select box from single to multiple or the other way around
at runtime is currently not supported and the two-way binding does odd stuff
when such situation happens.

we might eventually support this, but for now we are just going to not allow
binding to select[multiple] to prevent people from relying on something that
doesn't work.

BREAKING CHANGE: binding to select[multiple] directly or via ngMultiple (ng-multiple)
directive is not supported. This feature never worked with two-way data-binding,
so it's not expected that anybody actually depends on it.

Closes angular#3230
  • Loading branch information
IgorMinar committed Jul 25, 2013
1 parent 0fcd1e3 commit 45affbe
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 46 deletions.
5 changes: 5 additions & 0 deletions src/ng/compile.js
Expand Up @@ -1220,6 +1220,11 @@ function $CompileProvider($provide) {
if (!interpolateFn) return;


if (name === "multiple" && nodeName_(node) === "SELECT") {
throw new $compileMinErr("selmulti", "Binding to the multiple attribute is not supported. Element: {0}",
startingTag(node));
}

directives.push({
priority: 100,
compile: valueFn(function attrInterpolateLinkFn(scope, element, attr) {
Expand Down
39 changes: 3 additions & 36 deletions src/ng/directive/booleanAttrs.js
Expand Up @@ -199,42 +199,6 @@
*/


/**
* @ngdoc directive
* @name ng.directive:ngMultiple
* @restrict A
*
* @description
* The HTML specs do not require browsers to preserve the special attributes such as multiple.
* (The presence of them means true and absence means false)
* This prevents the angular compiler from correctly retrieving the binding expression.
* To solve this problem, we introduce the `ngMultiple` directive.
*
* @example
<doc:example>
<doc:source>
Check me check multiple: <input type="checkbox" ng-model="checked"><br/>
<select id="select" ng-multiple="checked">
<option>Misko</option>
<option>Igor</option>
<option>Vojta</option>
<option>Di</option>
</select>
</doc:source>
<doc:scenario>
it('should toggle multiple', function() {
expect(element('.doc-example-live #select').prop('multiple')).toBeFalsy();
input('checked').check();
expect(element('.doc-example-live #select').prop('multiple')).toBeTruthy();
});
</doc:scenario>
</doc:example>
*
* @element SELECT
* @param {expression} ngMultiple Angular expression that will be evaluated.
*/


/**
* @ngdoc directive
* @name ng.directive:ngReadonly
Expand Down Expand Up @@ -334,6 +298,9 @@ var ngAttributeAliasDirectives = {};

// boolean attrs are evaluated
forEach(BOOLEAN_ATTR, function(propName, attrName) {
// binding to multiple is not supported
if (propName == "multiple") return;

var normalized = directiveNormalize('ng-' + attrName);
ngAttributeAliasDirectives[normalized] = function() {
return {
Expand Down
34 changes: 24 additions & 10 deletions test/ng/directive/booleanAttrsSpec.js
Expand Up @@ -65,16 +65,6 @@ describe('boolean attr directives', function() {
}));


it('should bind multiple', inject(function($rootScope, $compile) {
element = $compile('<select ng-multiple="isMultiple"></select>')($rootScope)
$rootScope.isMultiple=false;
$rootScope.$digest();
expect(element.attr('multiple')).toBeFalsy();
$rootScope.isMultiple='multiple';
$rootScope.$digest();
expect(element.attr('multiple')).toBeTruthy();
}));

it('should bind open', inject(function($rootScope, $compile) {
element = $compile('<details ng-open="isOpen"></details>')($rootScope)
$rootScope.isOpen=false;
Expand All @@ -84,6 +74,30 @@ describe('boolean attr directives', function() {
$rootScope.$digest();
expect(element.attr('open')).toBeTruthy();
}));


describe('multiple', function() {
it('should NOT bind to multiple via ngMultiple', inject(function($rootScope, $compile) {
element = $compile('<select ng-multiple="isMultiple"></select>')($rootScope)
$rootScope.isMultiple=false;
$rootScope.$digest();
expect(element.attr('multiple')).toBeFalsy();
$rootScope.isMultiple='multiple';
$rootScope.$digest();
expect(element.attr('multiple')).toBeFalsy(); // ignore
}));


it('should throw an exception if binding to multiple attribute', inject(function($rootScope, $compile) {
if (msie < 9) return; //IE8 doesn't support biding to boolean attributes

expect(function() {
$compile('<select multiple="{{isMultiple}}"></select>')
}).toThrow('[$compile:selmulti] Binding to the multiple attribute is not supported. ' +
'Element: <select multiple="{{isMultiple}}">');

}));
});
});


Expand Down

0 comments on commit 45affbe

Please sign in to comment.