reiern70 commented on code in PR #1651:
URL: https://github.com/apache/wicket/pull/1651#discussion_r4193986811
##########
wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java:
##########
@@ -551,7 +554,7 @@ public final CharSequence getJsForListenerUrl(CharSequence
url)
}
buffer.append(String.format("var f =
document.getElementById('%s');", root.getMarkupId()));
buffer.append(String.format("f.action='%s';", action));
- buffer.append("Wicket.Event.fire(f, 'submit');");
+ buffer.append("if (Wicket.Event.triggerSubmit(f)) { f.submit();
}");
Review Comment:
`f.submit()` throws when the form contains a control named `submit`: a form
control's name shadows the form's property, so `f.submit` is that element, not
the method. In Wicket a control's `name` is its component path, so a button or
field with the id `submit` directly in the form is enough (a common id;
`AjaxFormSubmitBehaviorTest` uses it too). A `FormComponentUpdatingBehavior`
then fails with `TypeError: f.submit is not a function` and the form is not
submitted, with either Ajax engine. Before, the jQuery engine's `trigger()`
only called the native method when it was a function.
Suggestion: `HTMLFormElement.prototype.submit.call(f);`
##########
wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax.js:
##########
@@ -2785,6 +2785,47 @@
}
},
+ /**
+ * Fires a submit event on the given form, so that its submit
handlers run, without
+ * submitting the form: an Ajax request submits it instead.
Firefox submits a form for
+ * a submit event fired by a script unless the event is
cancelled, so the event is
+ * cancelled once the handlers on the form have run, as the
last listener on the form.
+ *
+ * @param form {HTMLFormElement} the form
+ * @returns {boolean} false if a handler on the form, or one
capturing the event on its
+ * way there, cancelled the event
+ */
+ triggerSubmit: function(form) {
+ const event = new Event('submit', { cancelable: true,
bubbles: true });
+ let cancelledByHandler = null;
+ const cancel = function () {
+ if (cancelledByHandler === null) {
+ cancelledByHandler =
event.defaultPrevented;
+ event.preventDefault();
+ }
+ };
+ const cancelOwnEvent = function (e) {
+ if (e === event) {
+ cancel();
+ }
+ };
+ // a handler stopping the immediate propagation keeps
cancelOwnEvent from running
+ const stopImmediatePropagation =
event.stopImmediatePropagation;
Review Comment:
Only `stopImmediatePropagation()` is covered. A capturing `submit` listener
on an ancestor (`document`, `body`) that calls `event.stopPropagation()`
without cancelling keeps the event from reaching the form, so `cancelOwnEvent`
never runs and the event is never cancelled: `triggerSubmit` returns `true`,
the Ajax request goes out, and Firefox submits the form as well, the double
submission this PR is about. Wrapping `event.stopPropagation` the same way
would close it (with the caveat that other listeners on that same element still
run afterwards and could no longer cancel the request).
##########
wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js:
##########
@@ -2505,6 +2505,47 @@
}
},
+ /**
+ * Fires a submit event on the given form, so that its
submit handlers run, without
+ * submitting the form: an Ajax request submits it
instead. Firefox submits a form for
+ * a submit event fired by a script unless the event is
cancelled, so the event is
+ * cancelled once the handlers on the form have run, as
the last listener on the form.
+ *
+ * @param form {HTMLFormElement} the form
+ * @returns {boolean} false if a handler on the form,
or one capturing the event on its
+ * way there, cancelled the event
+ */
+ triggerSubmit: function(form) {
+ const event = new Event('submit', { cancelable:
true, bubbles: true });
+ let cancelledByHandler = null;
+ const cancel = function () {
+ if (cancelledByHandler === null) {
+ cancelledByHandler =
event.defaultPrevented;
+ event.preventDefault();
+ }
+ };
+ const cancelOwnEvent = function (e) {
+ if (e === event) {
+ cancel();
+ }
+ };
+ // a handler stopping the immediate propagation
keeps cancelOwnEvent from running
+ const stopImmediatePropagation =
event.stopImmediatePropagation;
Review Comment:
Same as in wicket-ajax.js: `stopPropagation()` by a capturing ancestor isn't
covered, so Firefox still submits the form besides the Ajax request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
