Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Form leak fix #4779

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/ng/directive/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ var VALID_CLASS = 'ng-valid',
* When the directive updates the model value, calling `ngModel.$setViewValue()` the property
* on the outer scope will not be updated. However you can get around this by using $parent.
*
* Here is an example of this situation. You'll notice that the first div is not updating the input.
* Here is an example of this situation. You'll notice that the first div is not updating the input.
* However the second div can update the input properly.
*
* <example module="badIsolatedDirective">
Expand Down Expand Up @@ -1222,7 +1222,7 @@ var ngModelDirective = function() {

formCtrl.$addControl(modelCtrl);

element.on('$destroy', function() {
scope.$on('$destroy', function() {
formCtrl.$removeControl(modelCtrl);
});
}
Expand Down
30 changes: 19 additions & 11 deletions src/ng/directive/ngIf.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,31 @@ var ngIfDirective = ['$animate', function($animate) {
$$tlb: true,
compile: function (element, attr, transclude) {
return function ($scope, $element, $attr) {
var block = {}, childScope;
var block, childScope;
$scope.$watch($attr.ngIf, function ngIfWatchAction(value) {
if (block.startNode) {
$animate.leave(getBlockElements(block));
block = {};
}
if (block.startNode) {
getBlockElements(block).$destroy();
block = {};
}

if (toBoolean(value)) {

childScope = $scope.$new();
transclude(childScope, function (clone) {
block.startNode = clone[0];
block.endNode = clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ');
block = {
startNode: clone[0],
endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
};
$animate.enter(clone, $element.parent(), $element);
});

} else {

if (childScope) {
childScope.$destroy();
childScope = null;
}

if (block) {
$animate.leave(getBlockElements(block));
block = null;
}
}
});
};
Expand Down
21 changes: 15 additions & 6 deletions test/ng/directive/formSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,23 @@ describe('form', function() {
});


it('should remove the widget when element removed', function() {
it('should remove form control references from the form when nested control is removed from the DOM', function() {
doc = $compile(
'<form name="myForm">' +
'<input type="text" name="alias" ng-model="value" store-model-ctrl/>' +
'<input ng-if="inputPresent" name="alias" ng-model="value" store-model-ctrl/>' +
'</form>')(scope);
scope.inputPresent = true;
scope.$digest();

var form = scope.myForm;
control.$setValidity('required', false);
expect(form.alias).toBe(control);
expect(form.$error.required).toEqual([control]);

doc.find('input').remove();
// remove nested control
scope.inputPresent = false;
scope.$apply();

expect(form.$error.required).toBe(false);
expect(form.alias).toBeUndefined();
});
Expand Down Expand Up @@ -362,14 +367,15 @@ describe('form', function() {
});


it('should deregister a input when its removed from DOM', function() {
it('should deregister a input when it is removed from DOM', function() {
doc = jqLite(
'<form name="parent">' +
'<div class="ng-form" name="child">' +
'<input ng:model="modelA" name="inputA" required>' +
'<input ng-if="inputPresent" ng-model="modelA" name="inputA" required>' +
'</div>' +
'</form>');
$compile(doc)(scope);
scope.inputPresent = true;
scope.$apply();

var parent = scope.parent,
Expand All @@ -384,7 +390,10 @@ describe('form', function() {
expect(doc.hasClass('ng-invalid-required')).toBe(true);
expect(doc.find('div').hasClass('ng-invalid')).toBe(true);
expect(doc.find('div').hasClass('ng-invalid-required')).toBe(true);
doc.find('input').remove(); //remove child

//remove child input
scope.inputPresent = false;
scope.$apply();

expect(parent.$error.required).toBe(false);
expect(child.$error.required).toBe(false);
Expand Down
89 changes: 78 additions & 11 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,84 @@ describe('ngModel', function() {
expect(element).toBeInvalid();
expect(element).toHaveClass('ng-invalid-required');
}));


it('should register/deregister a nested ngModel with parent form when entering or leaving DOM',
inject(function($compile, $rootScope) {

var element = $compile('<form name="myForm">' +
'<input ng-if="inputPresent" name="myControl" ng-model="value" required >' +
'</form>')($rootScope);
var isFormValid;

$rootScope.inputPresent = false;
$rootScope.$watch('myForm.$valid', function(value) { isFormValid = value; });

$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

$rootScope.inputPresent = true;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(false);
expect(isFormValid).toBe(false);
expect($rootScope.myForm.myControl).toBeDefined();

$rootScope.inputPresent = false;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

dealoc(element);
}));


it('should register/deregister a nested ngModel with parent form when entering or leaving DOM with animations',
function() {

// ngAnimate performs the dom manipulation after digest, and since the form validity can be affected by a form
// control going away we must ensure that the deregistration happens during the digest while we are still doing
// dirty checking.
module('ngAnimate');

inject(function($compile, $rootScope) {
var element = $compile('<form name="myForm">' +
'<input ng-if="inputPresent" name="myControl" ng-model="value" required >' +
'</form>')($rootScope);
var isFormValid;

$rootScope.inputPresent = false;
// this watch ensure that the form validity gets updated during digest (so that we can observe it)
$rootScope.$watch('myForm.$valid', function(value) { isFormValid = value; });

$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

$rootScope.inputPresent = true;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(false);
expect(isFormValid).toBe(false);
expect($rootScope.myForm.myControl).toBeDefined();

$rootScope.inputPresent = false;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

dealoc(element);
});
});
});


Expand Down Expand Up @@ -369,17 +447,6 @@ describe('input', function() {
});


it('should cleanup it self from the parent form', function() {
compileInput('<input ng-model="name" name="alias" required>');

scope.$apply();
expect(scope.form.$error.required.length).toBe(1);

inputElm.remove();
expect(scope.form.$error.required).toBe(false);
});


it('should update the model on "blur" event', function() {
compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');

Expand Down
22 changes: 21 additions & 1 deletion test/ng/directive/ngIfSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('ngIf', function () {
expect(element.children().length).toBe(0);
});

it('should create a new scope', function () {
it('should create a new scope every time the expression evaluates to true', function () {
$scope.$apply('value = true');
element.append($compile(
'<div ng-if="value"><span ng-init="value=false"></span></div>'
Expand All @@ -45,6 +45,26 @@ describe('ngIf', function () {
expect(element.children('div').length).toBe(1);
});

it('should destroy the child scope every time the expression evaluates to false', function() {
$scope.value = true;
element.append($compile(
'<div ng-if="value"></div>'
)($scope));
$scope.$apply();

var childScope = element.children().scope();
var destroyed = false;

childScope.$on('$destroy', function() {
destroyed = true;
});

$scope.value = false;
$scope.$apply();

expect(destroyed).toBe(true);
});

it('should play nice with other elements beside it', function () {
$scope.values = [1, 2, 3, 4];
element.append($compile(
Expand Down