From a0fa4d2743ef0d96c2c58ebf2d6d14bd37188859 Mon Sep 17 00:00:00 2001 From: Johannes Raggam Date: Sat, 19 Aug 2023 15:57:02 +0200 Subject: [PATCH] fix(pat-inject): Don't submit forms with invalid data. Fix a problem with pat-inject and pat-validation where forms with invalid data could be submitted and the submit button wasn't inactive. The problem was fixed in two ways: - pat-inject now has a check for browser-native form validation. Invalid forms would not be submitted. - pat-inject now waits a tick before it get's initialized. Modern BasePattern based patterns including pat-validation are all deferred for 1 tick by design. pat-inject, being and older Pattern isn't deferred and thus initialized before pat-inject. It's initialized before pat-validation even though we have some Pattern initialization reorganization code in place - pat-inject not being deferred will have it's event handlers always run before any others. But now, with the 1-tick defer in place, pat-inject's event handlers are run in the correct order - after pat-validation form validation which in case of an invalid form would also deactivate the submit button. --- src/pat/inject/inject.js | 101 ++++++++------ src/pat/inject/inject.test.js | 191 ++++++++++++++++++++------ src/pat/modal/modal.js | 8 +- src/pat/modal/modal.test.js | 33 ++--- src/pat/navigation/navigation.test.js | 1 + src/pat/sortable/sortable.test.js | 1 + 6 files changed, 230 insertions(+), 105 deletions(-) diff --git a/src/pat/inject/inject.js b/src/pat/inject/inject.js index f8b4a8c2d..38d6e46aa 100644 --- a/src/pat/inject/inject.js +++ b/src/pat/inject/inject.js @@ -1,4 +1,5 @@ import "../../core/jquery-ext"; // for findInclusive +import "../../core/polyfills"; // SubmitEvent.submitter for Safari < 15.4 and jsDOM import $ from "jquery"; import ajax from "../ajax/ajax"; import dom from "../../core/dom"; @@ -47,7 +48,14 @@ const inject = { trigger: "a.pat-inject, form.pat-inject, .pat-subform.pat-inject", parser: parser, - init($el, opts) { + async init($el, opts) { + // We need to wait a tick. Modern BasePattern based patterns like + // pat-validation do always wait a tick before initializing. The + // patterns registry always initializes pat-validation first but since + // it is waiting a tick the event handlers are registerd after the ones + // from pat-inject. Waiting a tick in pat-inject solves this - + // pat-validation's event handlers are initialized first. + await utils.timeout(1); const cfgs = this.extractConfig($el, opts); if (cfgs.some((e) => e.history === "record") && !("pushState" in history)) { // if the injection shall add a history entry and HTML5 pushState @@ -95,21 +103,29 @@ const inject = { } }); // setup event handlers - if ($el.is("form")) { - $el.on("submit.pat-inject", this.onTrigger.bind(this)) - .on( - "click.pat-inject", - `[type=submit]:not([formaction]), - button:not([formaction]):not([type=button])`, - ajax.onClickSubmit - ) - .on( - "click.pat-inject", - `[type=submit][formaction], - [type=image][formaction], - button[formaction]:not([type=button])`, - this.onFormActionSubmit.bind(this) + if ($el[0]?.tagName === "FORM") { + events.add_event_listener( + $el[0], + "submit", + "pat-inject--form-submit", + (e) => { + this.onTrigger(e); + } + ); + for (const button of $el[0].querySelectorAll( + "[type=submit], button:not([type=button]), [type=image]" + )) { + events.add_event_listener( + button, + "click", + "pat-inject--form-submit-click", + (e) => { + // make sure the submitting button is sent + // with the form + ajax.onClickSubmit(e); + } ); + } } else if ($el.is(".pat-subform")) { log.debug("Initializing subform with injection"); } else { @@ -165,37 +181,40 @@ const inject = { /* Injection has been triggered, either via form submission or a * link has been clicked. */ - const $el = $(e.currentTarget); - const cfgs = $el.data("pat-inject"); - if ($el.is("form")) { - $(cfgs).each((i, v) => { - v.params = $.param($el.serializeArray()); - }); - } + + // Prevent the original event from doing it's work. + // We want an AJAX request instead. e.preventDefault && e.preventDefault(); - $el.trigger("patterns-inject-triggered"); - this.execute(cfgs, $el); - }, - onFormActionSubmit(e) { - ajax.onClickSubmit(e); // make sure the submitting button is sent with the form + const $el = $(e.currentTarget); + let cfgs = $el.data("pat-inject"); + if ($el[0].tagName === "FORM" && e.type === "submit") { + if ($el[0].matches(":invalid")) { + // Do not submit invalid forms. + // Works with native form validation and with pat-validation. + log.debug("Form is invalid, aborting"); + return; + } - const $button = $(e.target); - const formaction = $button.attr("formaction"); - const $form = $button.parents(".pat-inject").first(); - const opts = { - url: formaction, - }; - const $cfg_node = $button.closest("[data-pat-inject]"); - const cfgs = this.extractConfig($cfg_node, opts); + const submitter = e.submitter; + const formaction = submitter?.getAttribute("formaction"); + if (formaction) { + const opts = { + url: formaction, + }; - $(cfgs).each((i, v) => { - v.params = $.param($form.serializeArray()); - }); + // Support custom data-pat-inject on formaction buttons. + const cfg_node = submitter.closest("[data-pat-inject]"); + cfgs = this.extractConfig($(cfg_node), opts); + } - e.preventDefault(); - $form.trigger("patterns-inject-triggered"); - this.execute(cfgs, $form); + for (const cfg of cfgs) { + cfg.params = $.param($el.serializeArray()); + } + } + + $el.trigger("patterns-inject-triggered"); + this.execute(cfgs, $el); }, submitSubform($sub) { diff --git a/src/pat/inject/inject.test.js b/src/pat/inject/inject.test.js index 530e0ad13..7b19bb304 100644 --- a/src/pat/inject/inject.test.js +++ b/src/pat/inject/inject.test.js @@ -1,5 +1,6 @@ import pattern from "./inject"; import $ from "jquery"; +import events from "../../core/events"; import registry from "../../core/registry"; import utils from "../../core/utils"; import { jest } from "@jest/globals"; @@ -62,6 +63,8 @@ describe("pat-inject", function () { $(document).on("patterns-injected", callback); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); var cfgs = pattern.extractConfig($a, {}); @@ -86,6 +89,8 @@ describe("pat-inject", function () { $("#lab").empty().append($a).append($div); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); var cfgs = pattern.extractConfig($a, {}); @@ -100,14 +105,17 @@ describe("pat-inject", function () { spy_ajax.mockRestore(); }); - it("2.3 - can be set to an empty string value so that nothing gets added to the target while content is still loading'", function () { + it("2.3 - can be set to an empty string value so that nothing gets added to the target while content is still loading'", async function () { const spy_ajax = jest.spyOn($, "ajax"); var $a = $( 'link' ); var $div = $('
'); $("#lab").empty().append($a).append($div); + pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); var cfgs = pattern.extractConfig($a, {}); expect(cfgs[0].loadingClass).toBe(""); @@ -118,7 +126,7 @@ describe("pat-inject", function () { }); describe("3 - The confirm argument", function () { - it("3.1 - is by default set to 'class', which means it asks for confirmation based on a class on the target", function () { + it("3.1 - is by default set to 'class', which means it asks for confirmation based on a class on the target", async function () { var $a = $('link'); var $div = $('
'); $("#lab").empty().append($a).append($div); @@ -133,6 +141,8 @@ describe("pat-inject", function () { // Test that confirm doesn't get called pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); expect(spy_confirm).not.toHaveBeenCalled(); @@ -144,7 +154,7 @@ describe("pat-inject", function () { spy_confirm.mockRestore(); }); - it("3.2 - can be set to 'never' to never ask for confirmation", function () { + it("3.2 - can be set to 'never' to never ask for confirmation", async function () { var $a = $( 'link' ); @@ -161,6 +171,8 @@ describe("pat-inject", function () { // Test that confirm doesn't get called pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); expect(spy_confirm).not.toHaveBeenCalled(); expect(spy_onTrigger).toHaveBeenCalled(); @@ -169,7 +181,7 @@ describe("pat-inject", function () { spy_confirm.mockRestore(); }); - it("3.3 - can be set to 'always' to always ask for confirmation before injecting", function () { + it("3.3 - can be set to 'always' to always ask for confirmation before injecting", async function () { var $a = $( 'link' ); @@ -186,6 +198,8 @@ describe("pat-inject", function () { // Test that confirm does get called pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); expect(spy_onTrigger).toHaveBeenCalled(); expect(spy_confirm).toHaveBeenCalled(); @@ -194,7 +208,7 @@ describe("pat-inject", function () { spy_confirm.mockRestore(); }); - it("3.4 - can be set to 'form-data' to ask for confirmation before injecting over form fields changed by the user", function () { + it("3.4 - can be set to 'form-data' to ask for confirmation before injecting over form fields changed by the user", async function () { var $a = $( 'link' ); @@ -213,6 +227,8 @@ describe("pat-inject", function () { // Test that confirm does get called pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); expect(spy_confirm).toHaveBeenCalled(); @@ -226,7 +242,7 @@ describe("pat-inject", function () { }); describe("3.5 - The confirm-message argument", function () { - it("3.5.1 - can be used to provide a custom confirmation prompt message", function () { + it("3.5.1 - can be used to provide a custom confirmation prompt message", async function () { var $a = $( 'link' ); @@ -243,6 +259,8 @@ describe("pat-inject", function () { // Test that confirm doesn't get called pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); expect(spy_confirm).toHaveBeenCalled(); expect(spy_confirm).toHaveBeenCalledWith("Hello world"); @@ -577,7 +595,10 @@ describe("pat-inject", function () { const callback = jest.fn(); const inject_el = document.querySelector(".pat-inject"); inject_el.addEventListener("pat-inject-success", callback); + registry.scan(document.body); + await utils.timeout(1); // wait a tick for async to settle. + inject_el.click(); answer("
"); await utils.timeout(1); // wait a tick for async to settle. @@ -593,7 +614,10 @@ describe("pat-inject", function () { $("#lab").empty().append($a).append($div); var callback = jest.fn(); $(document).on("patterns-injected", callback); + pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer("" + '
repl
' + ""); await utils.timeout(1); // wait a tick for async to settle. @@ -609,7 +633,10 @@ describe("pat-inject", function () { $("#lab").empty().append($a).append($div); var callback = jest.fn(); $(document).on("patterns-injected-scanned", callback); + pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer(` @@ -633,7 +660,10 @@ describe("pat-inject", function () { $("#lab").empty().append($a).append($div); var callback = jest.fn(); $(document.body).on("patterns-injected-delayed", callback); + pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer(` @@ -659,7 +689,10 @@ describe("pat-inject", function () { $("#lab").empty().append($a).append($div); var callback = jest.fn(); $(document).on("patterns-injected-scanned", callback); + pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer(` @@ -683,7 +716,10 @@ describe("pat-inject", function () { $("#lab").empty().append($a).append($div); var callback = jest.fn(); $(document).on("patterns-injected-scanned", callback); + pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer(` @@ -719,30 +755,38 @@ describe("pat-inject", function () { spy_ajax.mockRestore(); }); - it("9.1.1 - fetches url on click", function () { + it("9.1.1 - fetches url on click", async function () { pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); expect($.ajax).toHaveBeenCalled(); expect($.ajax.mock.calls.pop()[0].url).toBe("test.html"); }); - it("9.1.2 - fetches url on autoload", function () { + it("9.1.2 - fetches url on autoload", async function () { $a.attr("data-pat-inject", "autoload"); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + expect($.ajax).toHaveBeenCalled(); expect($.ajax.mock.calls.pop()[0].url).toBe("test.html"); }); - it("9.1.3 - fetches url on autoload-delayed", function () { + it("9.1.3 - fetches url on autoload-delayed", async function () { $a.attr("data-pat-inject", "autoload-delayed"); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + // this needs to be checked async - is beyond me // expect($.ajax).toHaveBeenCalled(); // expect($.ajax.mock.calls.pop()[0].url).toBe("test.html"); }); - it("9.1.4 - fetches url on push_marker sent", function () { + it("9.1.4 - fetches url on push_marker sent", async function () { $a.attr("data-pat-inject", "push-marker: content-updated"); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $("body").trigger("push", ["content-updated"]); expect($.ajax).toHaveBeenCalled(); expect($.ajax.mock.calls.pop()[0].url).toBe("test.html"); @@ -750,6 +794,8 @@ describe("pat-inject", function () { it("9.1.5 - injects into existing div", async function () { pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer('
replacement
'); await utils.timeout(1); // wait a tick for async to settle. @@ -758,6 +804,8 @@ describe("pat-inject", function () { it("9.1.6 - injects multiple times", async function () { pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer('
replacement
'); await utils.timeout(1); // wait a tick for async to settle. @@ -789,6 +837,8 @@ describe("pat-inject", function () { $div.append($target1).append($target2); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer( "" + @@ -809,6 +859,8 @@ describe("pat-inject", function () { $div.append($target1).append($target2); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer( "" + '
repl
' + "" @@ -823,6 +875,8 @@ describe("pat-inject", function () { $a.attr("data-pat-inject", "#otherid::element #someid"); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer( "" + @@ -840,6 +894,8 @@ describe("pat-inject", function () { $div.append($('
')); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer( "" + @@ -862,6 +918,8 @@ describe("pat-inject", function () { $div.append($target1).append($target2); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer( "" + '
repl
' + "" @@ -877,6 +935,8 @@ describe("pat-inject", function () { $div.append($('
')); pattern.init($a); + await utils.timeout(1); // wait a tick for async to settle. + $a.trigger("click"); answer( "" + '
repl
' + "" @@ -904,6 +964,8 @@ describe("pat-inject", function () { it("9.2.1 - trigger injection on submit", async function () { pattern.init($form); + await utils.timeout(1); // wait a tick for async to settle. + $form.trigger("submit"); answer( "" + '
repl
' + "" @@ -913,11 +975,13 @@ describe("pat-inject", function () { expect($div.html()).toBe("repl"); }); - it("9.2.2 - pass get form parameters in ajax call as data", function () { + it("9.2.2 - pass get form parameters in ajax call as data", async function () { $form.attr("method", "get"); $form.append($('')); pattern.init($form); + await utils.timeout(1); // wait a tick for async to settle. + $form.trigger("submit"); var ajaxargs = $.ajax.mock.calls[$.ajax.mock.calls.length - 1][0]; @@ -925,11 +989,13 @@ describe("pat-inject", function () { expect(ajaxargs.data).toContain("param=somevalue"); }); - it("9.2.3 - pass post form parameters in ajax call as data", function () { + it("9.2.3 - pass post form parameters in ajax call as data", async function () { $form.attr("method", "post"); $form.append($('')); pattern.init($form); + await utils.timeout(1); // wait a tick for async to settle. + $form.trigger("submit"); var ajaxargs = $.ajax.mock.calls[$.ajax.mock.calls.length - 1][0]; @@ -937,13 +1003,15 @@ describe("pat-inject", function () { expect(ajaxargs.data.get("param")).toContain("somevalue"); }); - it("9.2.4 - pass submit button value in ajax call as data", function () { + it("9.2.4 - pass submit button value in ajax call as data", async function () { var $submit = $(''); $form.attr("method", "post"); $form.append($submit); pattern.init($form); + await utils.timeout(1); // wait a tick for async to settle. + $submit.trigger("click"); var ajaxargs = $.ajax.mock.calls[$.ajax.mock.calls.length - 1][0]; @@ -952,19 +1020,20 @@ describe("pat-inject", function () { }); describe("9.2.5 - formaction attribute on submit buttons", function () { - it("9.2.5.1 - use submit button formaction value as action URL", function () { + it("9.2.5.1 - use submit button formaction value as action URL", async function () { var $submit1 = $( '' ), $submit2 = $( - '' + '' ); - $submit2.attr("formaction", "other.html"); $form.append($submit1).append($submit2); pattern.init($form); - $submit2.trigger("click"); + await utils.timeout(1); // wait a tick for async to settle. + + $submit2[0].click(); var ajaxargs = $.ajax.mock.calls[$.ajax.mock.calls.length - 1][0]; expect($.ajax).toHaveBeenCalled(); @@ -972,19 +1041,25 @@ describe("pat-inject", function () { expect(ajaxargs.data).toBe("submit=special"); }); - it("9.2.5.2 - use an image submit with a formaction value as action URL", function () { + it("9.2.5.2 - use an image submit with a formaction value as action URL", async function () { var $submit1 = $( '' ), $submit2 = $( - '' + '' ); - $submit2.attr("formaction", "other.html"); $form.append($submit1).append($submit2); pattern.init($form); - $submit2.trigger("click"); + await utils.timeout(1); // wait a tick for async to settle. + + // Work around jsDOM not submitting with image buttons. + $submit2[0].addEventListener("click", () => { + $submit2[0].form.dispatchEvent(events.submit_event()); + }); + + $submit2[0].click(); var ajaxargs = $.ajax.mock.calls[$.ajax.mock.calls.length - 1][0]; expect($.ajax).toHaveBeenCalled(); @@ -997,16 +1072,18 @@ describe("pat-inject", function () { '' ), $submit2 = $( - '' + '' ), $target = $('
'); - $submit2.attr("formaction", "other.html#otherid"); $form.append($submit1).append($submit2); $div.append($target); pattern.init($form); - $submit2.trigger("click"); + await utils.timeout(1); // wait a tick for async to settle. + + $submit2[0].click(); + answer( "" + '
other
' + @@ -1026,17 +1103,19 @@ describe("pat-inject", function () { '' ), $submit2 = $( - '' + '' ), $target = $('
'); $form.attr("data-pat-inject", "target: #othertarget"); - $submit2.attr("formaction", "other.html#otherid"); $form.append($submit1).append($submit2); $div.append($target); pattern.init($form); - $submit2.trigger("click"); + await utils.timeout(1); // wait a tick for async to settle. + + $submit2[0].click(); + answer( "" + '
other
' + @@ -1056,7 +1135,7 @@ describe("pat-inject", function () { '' ), $submit2 = $( - '' + '' ), $target1 = $('
'), $target2 = $('
'); @@ -1065,12 +1144,14 @@ describe("pat-inject", function () { "data-pat-inject", "target: #target1 && target: #target2" ); - $submit2.attr("formaction", "other.html#otherid"); $form.append($submit1).append($submit2); $div.append($target1).append($target2); pattern.init($form); - $submit2.trigger("click"); + await utils.timeout(1); // wait a tick for async to settle. + + $submit2[0].click(); + answer( "" + '
other
' + @@ -1091,7 +1172,7 @@ describe("pat-inject", function () { '' ), $submit2 = $( - '' + '' ), $target1 = $('
'), $target2 = $('
'); @@ -1100,12 +1181,14 @@ describe("pat-inject", function () { "data-pat-inject", "#someid #target1 && #otherid #target2" ); - $submit2.attr("formaction", "other.html#otherid"); $form.append($submit1).append($submit2); $div.append($target1).append($target2); pattern.init($form); - $submit2.trigger("click"); + await utils.timeout(1); // wait a tick for async to settle. + + $submit2[0].click(); + answer( "" + '
some
' + @@ -1127,7 +1210,7 @@ describe("pat-inject", function () { '' ), $submit2 = $( - '' + '' ), $target1 = $('
'), $target2 = $('
'); @@ -1136,12 +1219,14 @@ describe("pat-inject", function () { "data-pat-inject", "#someid #target1 && #otherid #target2" ); - $submit2.attr("formaction", "other.html#otherid"); $form.append($submit1).append($submit2); $div.append($target1).append($target2); pattern.init($form); - $submit2.trigger("click"); + await utils.timeout(1); // wait a tick for async to settle. + + $submit2[0].click(); + answer( "" + '
some
' + @@ -1236,6 +1321,8 @@ describe("pat-inject", function () { const button = form.querySelector("button"); pattern.init($(form)); + await utils.timeout(1); // wait a tick for async to settle. + button.click(); expect(pat_ajax.onClickSubmit).toHaveBeenCalledTimes(1); @@ -1252,19 +1339,21 @@ describe("pat-inject", function () { const pat_ajax = (await import("../ajax/ajax.js")).default; jest.spyOn(pat_ajax, "onClickSubmit"); - jest.spyOn(pattern, "onFormActionSubmit"); + jest.spyOn(pattern, "onTrigger"); const form = document.querySelector("form"); const button = form.querySelector("button"); pattern.init($(form)); + await utils.timeout(1); // wait a tick for async to settle. + button.click(); expect(pat_ajax.onClickSubmit).toHaveBeenCalledTimes(1); - expect(pattern.onFormActionSubmit).toHaveBeenCalledTimes(0); + expect(pattern.onTrigger).toHaveBeenCalledTimes(1); }); - it("9.2.6.1 - ... with a formaction atttribute.", async function () { + it("9.2.6.2 - ... with a formaction atttribute.", async function () { document.body.innerHTML = `