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

Commit 8f989d6

Browse files
committed
fix(ngModel): deregister from the form on scope not DOM destruction
Due to animations, DOM might get destroyed much later than scope and so the element $destroy event might get fired outside of $digest, which causes changes to the validation model go unobserved until the next digest. By deregistering on scope event, the deregistration always happens in $digest and the form validation model changes will be observed. Closes #4226 Closes #4779
1 parent 9483373 commit 8f989d6

File tree

3 files changed

+94
-18
lines changed

3 files changed

+94
-18
lines changed

src/ng/directive/input.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,7 @@ var ngModelDirective = function() {
12221222

12231223
formCtrl.$addControl(modelCtrl);
12241224

1225-
element.on('$destroy', function() {
1225+
scope.$on('$destroy', function() {
12261226
formCtrl.$removeControl(modelCtrl);
12271227
});
12281228
}

test/ng/directive/formSpec.js

+15-6
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,23 @@ describe('form', function() {
3636
});
3737

3838

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

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

50-
doc.find('input').remove();
52+
// remove nested control
53+
scope.inputPresent = false;
54+
scope.$apply();
55+
5156
expect(form.$error.required).toBe(false);
5257
expect(form.alias).toBeUndefined();
5358
});
@@ -362,14 +367,15 @@ describe('form', function() {
362367
});
363368

364369

365-
it('should deregister a input when its removed from DOM', function() {
370+
it('should deregister a input when it is removed from DOM', function() {
366371
doc = jqLite(
367372
'<form name="parent">' +
368373
'<div class="ng-form" name="child">' +
369-
'<input ng:model="modelA" name="inputA" required>' +
374+
'<input ng-if="inputPresent" ng-model="modelA" name="inputA" required>' +
370375
'</div>' +
371376
'</form>');
372377
$compile(doc)(scope);
378+
scope.inputPresent = true;
373379
scope.$apply();
374380

375381
var parent = scope.parent,
@@ -384,7 +390,10 @@ describe('form', function() {
384390
expect(doc.hasClass('ng-invalid-required')).toBe(true);
385391
expect(doc.find('div').hasClass('ng-invalid')).toBe(true);
386392
expect(doc.find('div').hasClass('ng-invalid-required')).toBe(true);
387-
doc.find('input').remove(); //remove child
393+
394+
//remove child input
395+
scope.inputPresent = false;
396+
scope.$apply();
388397

389398
expect(parent.$error.required).toBe(false);
390399
expect(child.$error.required).toBe(false);

test/ng/directive/inputSpec.js

+78-11
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,84 @@ describe('ngModel', function() {
305305
expect(element).toBeInvalid();
306306
expect(element).toHaveClass('ng-invalid-required');
307307
}));
308+
309+
310+
it('should register/deregister a nested ngModel with parent form when entering or leaving DOM',
311+
inject(function($compile, $rootScope) {
312+
313+
var element = $compile('<form name="myForm">' +
314+
'<input ng-if="inputPresent" name="myControl" ng-model="value" required >' +
315+
'</form>')($rootScope);
316+
var isFormValid;
317+
318+
$rootScope.inputPresent = false;
319+
$rootScope.$watch('myForm.$valid', function(value) { isFormValid = value; });
320+
321+
$rootScope.$apply();
322+
323+
expect($rootScope.myForm.$valid).toBe(true);
324+
expect(isFormValid).toBe(true);
325+
expect($rootScope.myForm.myControl).toBeUndefined();
326+
327+
$rootScope.inputPresent = true;
328+
$rootScope.$apply();
329+
330+
expect($rootScope.myForm.$valid).toBe(false);
331+
expect(isFormValid).toBe(false);
332+
expect($rootScope.myForm.myControl).toBeDefined();
333+
334+
$rootScope.inputPresent = false;
335+
$rootScope.$apply();
336+
337+
expect($rootScope.myForm.$valid).toBe(true);
338+
expect(isFormValid).toBe(true);
339+
expect($rootScope.myForm.myControl).toBeUndefined();
340+
341+
dealoc(element);
342+
}));
343+
344+
345+
it('should register/deregister a nested ngModel with parent form when entering or leaving DOM with animations',
346+
function() {
347+
348+
// ngAnimate performs the dom manipulation after digest, and since the form validity can be affected by a form
349+
// control going away we must ensure that the deregistration happens during the digest while we are still doing
350+
// dirty checking.
351+
module('ngAnimate');
352+
353+
inject(function($compile, $rootScope) {
354+
var element = $compile('<form name="myForm">' +
355+
'<input ng-if="inputPresent" name="myControl" ng-model="value" required >' +
356+
'</form>')($rootScope);
357+
var isFormValid;
358+
359+
$rootScope.inputPresent = false;
360+
// this watch ensure that the form validity gets updated during digest (so that we can observe it)
361+
$rootScope.$watch('myForm.$valid', function(value) { isFormValid = value; });
362+
363+
$rootScope.$apply();
364+
365+
expect($rootScope.myForm.$valid).toBe(true);
366+
expect(isFormValid).toBe(true);
367+
expect($rootScope.myForm.myControl).toBeUndefined();
368+
369+
$rootScope.inputPresent = true;
370+
$rootScope.$apply();
371+
372+
expect($rootScope.myForm.$valid).toBe(false);
373+
expect(isFormValid).toBe(false);
374+
expect($rootScope.myForm.myControl).toBeDefined();
375+
376+
$rootScope.inputPresent = false;
377+
$rootScope.$apply();
378+
379+
expect($rootScope.myForm.$valid).toBe(true);
380+
expect(isFormValid).toBe(true);
381+
expect($rootScope.myForm.myControl).toBeUndefined();
382+
383+
dealoc(element);
384+
});
385+
});
308386
});
309387

310388

@@ -369,17 +447,6 @@ describe('input', function() {
369447
});
370448

371449

372-
it('should cleanup it self from the parent form', function() {
373-
compileInput('<input ng-model="name" name="alias" required>');
374-
375-
scope.$apply();
376-
expect(scope.form.$error.required.length).toBe(1);
377-
378-
inputElm.remove();
379-
expect(scope.form.$error.required).toBe(false);
380-
});
381-
382-
383450
it('should update the model on "blur" event', function() {
384451
compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');
385452

0 commit comments

Comments
 (0)