diff options
| -rw-r--r-- | src/ng/directive/form.js | 78 | ||||
| -rw-r--r-- | test/ng/directive/formSpec.js | 101 |
2 files changed, 146 insertions, 33 deletions
diff --git a/src/ng/directive/form.js b/src/ng/directive/form.js index cc229759..6b876e01 100644 --- a/src/ng/directive/form.js +++ b/src/ng/directive/form.js @@ -227,38 +227,62 @@ function FormController(element, attrs) { </doc:scenario> </doc:example> */ -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/ng/directive/formSpec.js b/test/ng/directive/formSpec.js index 96855f52..51c6929f 100644 --- a/test/ng/directive/formSpec.js +++ b/test/ng/directive/formSpec.js @@ -129,14 +129,28 @@ describe('form', function() { it('should prevent form submission', function() { var nextTurn = false, + submitted = false, reloadPrevented; - doc = jqLite('<form><input type="submit" value="submit" ></form>'); + doc = jqLite('<form ng-submit="submitMe()">' + + '<input type="submit" value="submit">' + + '</form>'); + + 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); - doc.bind('submit', function(e) { - reloadPrevented = e.defaultPrevented; - }); + scope.submitMe = function() { + submitted = true; + } + + if (msie!=8) addEventListenerFn(doc[0], 'submit', assertPreventDefaultListener); browserTrigger(doc.find('input')); @@ -147,17 +161,92 @@ describe('form', function() { runs(function() { expect(reloadPrevented).toBe(true); + expect(submitted).toBe(true); + + // prevent mem leak in test + removeEventListenerFn(doc[0], 'submit', assertPreventDefaultListener); }); }); - it('should not prevent form submission if action attribute present', function() { + it('should prevent the default when the form is destroyed by a submission via a click event', + inject(function($timeout) { + doc = jqLite('<div>' + + '<form ng-submit="submitMe()">' + + '<button ng-click="destroy()"></button>' + + '</form>' + + '</div>'); + + 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('<form name="x" action="some.py" />')(scope); + doc = $compile('<form action="some.py"></form>')(scope); doc.bind('submit', callback); browserTrigger(doc, 'submit'); |