Skip to content

Commit

Permalink
fix(remix-react): fix submitter serialization
Browse files Browse the repository at this point in the history
Bring `<Form>` submissions in line with the spec with respect to how and where
form submitters are serialized within the data set:

1. Ensure they are serialized in tree order (i.e. where they appear in the DOM)
2. Ensure image inputs are serialized correctly (i.e. separate x and y coordinate
   entries, rather than a single empty entry)
3. Don't send multiple entries in older WebKit

References: remix-run#4342
Spec: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-form-data-set
  • Loading branch information
jenseng committed Nov 1, 2022
1 parent b380d5e commit 5967ed1
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 11 deletions.
5 changes: 0 additions & 5 deletions integration/form-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -973,12 +973,7 @@ test.describe("Forms", () => {

test("submits the submitter's value(s) in tree order in the form data", async ({
page,
javaScriptEnabled,
}) => {
test.fail(
Boolean(javaScriptEnabled),
"<Form> doesn't serialize submit buttons correctly #4342"
);
let app = new PlaywrightFixture(appFixture, page);

await app.goto("/submitter");
Expand Down
74 changes: 68 additions & 6 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1201,12 +1201,7 @@ export function useSubmitImpl(key?: string): SubmitFunction {
target.getAttribute("formenctype") ||
form.getAttribute("enctype") ||
defaultEncType;
formData = new FormData(form);

// Include name + value from a <button>
if (target.name) {
formData.append(target.name, target.value);
}
formData = buildFormData(form, target);
} else {
if (isHtmlElement(target)) {
throw new Error(
Expand Down Expand Up @@ -1298,6 +1293,69 @@ export function useSubmitImpl(key?: string): SubmitFunction {

let nextNavigationSubmission: Submission | undefined;

// track the selected coordinate of an image button, since FormData can't do this (yet); see buildFormData
const SELECTED_COORDINATE = Symbol();
interface HTMLImageButtonElement extends HTMLInputElement {
[SELECTED_COORDINATE]?: { x: number; y: number };
}
// we only ever need one of these on the page, and we don't want it to go away
if (typeof document !== "undefined") {
document.body.addEventListener("click", (e) => {
if (isImageButtonElement(e.target)) {
e.target[SELECTED_COORDINATE] = { x: e.offsetX, y: e.offsetY };
}
});
}

/**
* Build the form data set
*
* FormData doesn't (yet) have a submitter-aware constructor -- see https://github.com/whatwg/xhr/issues/262
*
* In the meantime, we can temporarily tweak the form during to ensure the data set adheres to the spec:
* https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-form-data-set
*/
function buildFormData(
form: HTMLFormElement,
submitter: HTMLButtonElement | HTMLInputElement
) {
let tempFieldContainer = document.createElement("span");
submitter.insertAdjacentElement("afterend", tempFieldContainer);

let isExternalSubmitter = !form.contains(submitter);
function addSubmitterTempField(name: string, value: unknown) {
let field = document.createElement("input");
field.type = "hidden";
field.name = name;
field.value = String(value);
if (isExternalSubmitter) field.setAttribute("form", form.id);
tempFieldContainer.insertAdjacentElement("beforeend", field);
}

// disable the submitter, since some browsers (old Safari) unilaterally include it, and we don't want it twice 🙃
submitter.disabled = true;

// inject appropriate hidden field(s) next to the now disabled submitter 💪
if (isImageButtonElement(submitter)) {
if (submitter[SELECTED_COORDINATE]) {
let prefix = submitter.name ? `${submitter.name}.` : "";
addSubmitterTempField(`${prefix}x`, submitter[SELECTED_COORDINATE].x);
addSubmitterTempField(`${prefix}y`, submitter[SELECTED_COORDINATE].y);
}
} else if (submitter.name) {
addSubmitterTempField(submitter.name, submitter.value);
}

// ok now it should serialize per the spec 😅
let formData = new FormData(form);

// pretend none of this ever happened 🙈
tempFieldContainer.remove();
submitter.disabled = false;

return formData;
}

function setNextNavigationSubmission(submission: Submission) {
nextNavigationSubmission = submission;
}
Expand All @@ -1324,6 +1382,10 @@ function isInputElement(object: any): object is HTMLInputElement {
return isHtmlElement(object) && object.tagName.toLowerCase() === "input";
}

function isImageButtonElement(object: any): object is HTMLImageButtonElement {
return isInputElement(object) && object.type === "image";
}

/**
* Setup a callback to be fired on the window's `beforeunload` event. This is
* useful for saving some data to `window.localStorage` just before the page
Expand Down

0 comments on commit 5967ed1

Please sign in to comment.