diff --git a/src/ng/directive/form.js b/src/ng/directive/form.js index cc229759b9cb..6b876e01be0e 100644 --- a/src/ng/directive/form.js +++ b/src/ng/directive/form.js @@ -227,38 +227,62 @@ function FormController(element, attrs) { */ -var formDirectiveDir = { - name: 'form', - restrict: 'E', - controller: FormController, - compile: function() { - return { - pre: function(scope, formElement, attr, controller) { - if (!attr.action) { - formElement.bind('submit', function(event) { - event.preventDefault(); - }); - } +var formDirectiveFactory = function(isNgForm) { + return ['$timeout', function($timeout) { + var formDirective = { + name: 'form', + restrict: 'E', + controller: FormController, + compile: function() { + return { + pre: function(scope, formElement, attr, controller) { + if (!attr.action) { + // we can't use jq events because if a form is destroyed during submission the default + // action is not prevented. see #1238 + // + // IE 9 is not affected because it doesn't fire a submit event and try to do a full + // page reload if the form was destroyed by submission of the form via a click handler + // on a button in the form. Looks like an IE9 specific bug. + var preventDefaultListener = function(event) { + event.preventDefault + ? event.preventDefault() + : event.returnValue = false; // IE + }; - var parentFormCtrl = formElement.parent().controller('form'), - alias = attr.name || attr.ngForm; + addEventListenerFn(formElement[0], 'submit', preventDefaultListener); + + // unregister the preventDefault listener so that we don't not leak memory but in a + // way that will achieve the prevention of the default action. + formElement.bind('$destroy', function() { + $timeout(function() { + removeEventListenerFn(formElement[0], 'submit', preventDefaultListener); + }, 0, false); + }); + } + + var parentFormCtrl = formElement.parent().controller('form'), + alias = attr.name || attr.ngForm; - if (alias) { - scope[alias] = controller; - } - if (parentFormCtrl) { - formElement.bind('$destroy', function() { - parentFormCtrl.$removeControl(controller); if (alias) { - scope[alias] = undefined; + scope[alias] = controller; } - extend(controller, nullFormCtrl); //stop propagating child destruction handlers upwards - }); - } + if (parentFormCtrl) { + formElement.bind('$destroy', function() { + parentFormCtrl.$removeControl(controller); + if (alias) { + scope[alias] = undefined; + } + extend(controller, nullFormCtrl); //stop propagating child destruction handlers upwards + }); + } + } + }; } }; - } + + return isNgForm ? extend(copy(formDirective), {restrict: 'EAC'}) : formDirective; + }]; }; -var formDirective = valueFn(formDirectiveDir); -var ngFormDirective = valueFn(extend(copy(formDirectiveDir), {restrict: 'EAC'})); +var formDirective = formDirectiveFactory(); +var ngFormDirective = formDirectiveFactory(true); diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index 406b7a5af4ae..1a56a3437692 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -2,6 +2,9 @@ describe('jqLite', function() { var scope, a, b, c; + + beforeEach(module(provideLog)); + beforeEach(function() { a = jqLite('
A
')[0]; b = jqLite('
B
')[0]; @@ -241,7 +244,7 @@ describe('jqLite', function() { expect(jqLite(c).data('prop')).toBeUndefined(); }); - it('should call $destroy function if element removed', function() { + it('should emit $destroy event if element removed via remove()', function() { var log = ''; var element = jqLite(a); element.bind('$destroy', function() {log+= 'destroy;';}); @@ -249,6 +252,18 @@ describe('jqLite', function() { expect(log).toEqual('destroy;'); }); + + it('should emit $destroy event if an element is removed via html()', inject(function(log) { + var element = jqLite('
x
'); + element.find('span').bind('$destroy', log.fn('destroyed')); + + element.html(''); + + expect(element.html()).toBe(''); + expect(log).toEqual('destroyed'); + })); + + it('should retrieve all data if called without params', function() { var element = jqLite(a); expect(element.data()).toEqual({}); diff --git a/test/ng/directive/formSpec.js b/test/ng/directive/formSpec.js index 8b87f266c2cf..51c6929fc84d 100644 --- a/test/ng/directive/formSpec.js +++ b/test/ng/directive/formSpec.js @@ -64,36 +64,6 @@ describe('form', function() { }); - it('should prevent form submission', function() { - var startingUrl = '' + window.location; - doc = jqLite('
'); - $compile(doc)(scope); - - browserTrigger(doc.find('input')); - waitsFor( - function() { return true; }, - 'let browser breath, so that the form submision can manifest itself', 10); - - runs(function() { - expect('' + window.location).toEqual(startingUrl); - }); - }); - - - it('should not prevent form submission if action attribute present', function() { - var callback = jasmine.createSpy('submit').andCallFake(function(event) { - expect(event.isDefaultPrevented()).toBe(false); - event.preventDefault(); - }); - - doc = $compile('')(scope); - doc.bind('submit', callback); - - browserTrigger(doc, 'submit'); - expect(callback).toHaveBeenCalledOnce(); - }); - - it('should publish form to scope when name attr is defined', function() { doc = $compile('
')(scope); expect(scope.myForm).toBeTruthy(); @@ -155,6 +125,136 @@ describe('form', function() { }); + describe('preventing default submission', function() { + + it('should prevent form submission', function() { + var nextTurn = false, + submitted = false, + reloadPrevented; + + doc = jqLite('
' + + '' + + '
'); + + var assertPreventDefaultListener = function(e) { + reloadPrevented = e.defaultPrevented || (e.returnValue === false); + }; + + // native dom event listeners in IE8 fire in LIFO order so we have to register them + // there in different order than in other browsers + if (msie==8) addEventListenerFn(doc[0], 'submit', assertPreventDefaultListener); + + $compile(doc)(scope); + + scope.submitMe = function() { + submitted = true; + } + + if (msie!=8) addEventListenerFn(doc[0], 'submit', assertPreventDefaultListener); + + browserTrigger(doc.find('input')); + + // let the browser process all events (and potentially reload the page) + setTimeout(function() { nextTurn = true;}); + + waitsFor(function() { return nextTurn; }); + + runs(function() { + expect(reloadPrevented).toBe(true); + expect(submitted).toBe(true); + + // prevent mem leak in test + removeEventListenerFn(doc[0], 'submit', assertPreventDefaultListener); + }); + }); + + + it('should prevent the default when the form is destroyed by a submission via a click event', + inject(function($timeout) { + doc = jqLite('
' + + '
' + + '' + + '
' + + '
'); + + var form = doc.find('form'), + destroyed = false, + nextTurn = false, + submitted = false, + reloadPrevented; + + scope.destroy = function() { + // yes, I know, scope methods should not do direct DOM manipulation, but I wanted to keep + // this test small. Imagine that the destroy action will cause a model change (e.g. + // $location change) that will cause some directive to destroy the dom (e.g. ngView+$route) + doc.html(''); + destroyed = true; + } + + scope.submitMe = function() { + submitted = true; + } + + var assertPreventDefaultListener = function(e) { + reloadPrevented = e.defaultPrevented || (e.returnValue === false); + }; + + // native dom event listeners in IE8 fire in LIFO order so we have to register them + // there in different order than in other browsers + if (msie == 8) addEventListenerFn(form[0], 'submit', assertPreventDefaultListener); + + $compile(doc)(scope); + + if (msie != 8) addEventListenerFn(form[0], 'submit', assertPreventDefaultListener); + + browserTrigger(doc.find('button'), 'click'); + + // let the browser process all events (and potentially reload the page) + setTimeout(function() { nextTurn = true;}, 100); + + waitsFor(function() { return nextTurn; }); + + + // I can't get IE8 to automatically trigger submit in this test, in production it does it + // properly + if (msie == 8) browserTrigger(form, 'submit'); + + runs(function() { + expect(doc.html()).toBe(''); + expect(destroyed).toBe(true); + expect(submitted).toBe(false); // this is known corner-case that is not currently handled + // the issue is that the submit listener is destroyed before + // the event propagates there. we can fix this if we see + // the issue in the wild, I'm not going to bother to do it + // now. (i) + + // IE9 is special and it doesn't fire submit event when form was destroyed + if (msie != 9) { + expect(reloadPrevented).toBe(true); + $timeout.flush(); + } + + // prevent mem leak in test + removeEventListenerFn(form[0], 'submit', assertPreventDefaultListener); + }); + })); + + + it('should NOT prevent form submission if action attribute present', function() { + var callback = jasmine.createSpy('submit').andCallFake(function(event) { + expect(event.isDefaultPrevented()).toBe(false); + event.preventDefault(); + }); + + doc = $compile('
')(scope); + doc.bind('submit', callback); + + browserTrigger(doc, 'submit'); + expect(callback).toHaveBeenCalledOnce(); + }); + }); + + describe('nested forms', function() { it('should chain nested forms', function() {