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. We accomplish this by
constructing the form data set ourselves according to the spec.

Problems fixed:
1. Serialize submitters in tree order (i.e. where they appear in the DOM)
2. Serialize Image Button submitter correctly (i.e. separate x and y coordinate
   entries, rather than a single empty entry)
3. Stop sending 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
jenseng committed Nov 1, 2022
1 parent b380d5e commit 7b72226
Show file tree
Hide file tree
Showing 2 changed files with 282 additions and 11 deletions.
98 changes: 93 additions & 5 deletions integration/form-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,71 @@ test.describe("Forms", () => {
}
`,

"app/routes/form-data.jsx": js`
import { Form } from "@remix-run/react";
export default function() {
return (
<>
<Form action="/outputFormData" id="myform">
{/* the basics */}
<input type="text" name="text" defaultValue="hello" />
<input type="hidden" name="text" value="world" />
<textarea name="text" defaultValue="yay"></textarea>
<button id="go" name="go" value="go!" />
{/* radios/checkboxes/selects */}
<input type="radio" name="rad" value="1" />
<input type="radio" name="rad" value="2" defaultChecked />
<input type="radio" name="rad" value="3" />
<input type="checkbox" name="checky" value="1" defaultChecked />
<input type="checkbox" name="checky" value="2" defaultChecked />
<input type="checkbox" name="checky" value="3" />
<select name="selecty" defaultValue="2">
<option>1</option>
<option>2</option>
</select>
<select name="selecty2" multiple defaultValue={["2", "3"]}>
<option>1</option>
<option>2</option>
<option>3</option>
</select>
{/* unnamed */}
<input defaultValue="skipped" />
{/* various disabled things */}
<input name="input-disabled" disabled defaultValue="skipped" />
<select name="select-with-disabled-selected-option" defaultValue="1">
<option disabled>skipped</option>
</select>
<fieldset disabled>
<input name="fieldset-disabled-input" defaultValue="skipped" />
<legend>
{/* this is considered enabled, per the spec */}
<input
name="fieldset-disabled-legend-input-enabled"
defaultValue="1"
/>
</legend>
</fieldset>
{/* various form ownweship permutations */}
<input name="text" defaultValue="1" />
<input name="text" form="myform" defaultValue="2" />
<input name="text" form="unrelated" defaultValue="skipped" />
</Form>
<form id="unrelated">
<input name="text" defaultValue="skipped" />
<input name="text" form="myform" defaultValue="3" />
</form>
<input name="text" defaultValue="skipped" />
<input name="text" form="myform" defaultValue="4" />
</>
);
}
`,

"app/routes/file-upload.jsx": js`
import { Form, useSearchParams } from "@remix-run/react";
Expand Down Expand Up @@ -973,12 +1038,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 Expand Up @@ -1012,6 +1072,34 @@ test.describe("Forms", () => {
);
});

test("serializes form data correctly", async ({ page }) => {
// since we construct the form data set ourselves for <Form> submissions, test just
// about everything to ensure parity with <form> 😅
let app = new PlaywrightFixture(appFixture, page);

await app.goto("/form-data");
await app.clickElement("#go");
expect((await app.getElement("#formData")).val()).toBe(
new URLSearchParams([
["text", "hello"],
["text", "world"],
["text", "yay"],
["go", "go!"],
["rad", "2"],
["checky", "1"],
["checky", "2"],
["selecty", "2"],
["selecty2", "2"],
["selecty2", "3"],
["fieldset-disabled-legend-input-enabled", "1"],
["text", "1"],
["text", "2"],
["text", "3"],
["text", "4"],
]).toString()
);
});

test("sends file names when submitting via url encoding", async ({
page,
javaScriptEnabled,
Expand Down
195 changes: 189 additions & 6 deletions packages/remix-react/components.tsx
View file
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,182 @@ 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 follow the HTML spec and build the data set ourselves:
* https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-form-data-set
*
* Notes:
* - we return a FormData object rather than an entry list of tuples
* - we skip a few things in the spec that are deprecated/unimplemented in browsers (see inline
* IMPLEMENTATION NOTES)
*/
function buildFormData(
form: HTMLFormElement,
submitter: HTMLButtonElement | HTMLInputElement
) {
// 1. If form's constructing entry list is true, then return null.
// 2. Set form's constructing entry list to true.
// 3. Let controls be a list of all the submittable elements whose form owner is form, in tree order.
let submittable = ":is(button,input,select,textarea)";
type SubmittableElement =
| HTMLButtonElement
| HTMLInputElement
| HTMLSelectElement
| HTMLTextAreaElement;

let scope: HTMLElement | Document = form;
let ownedByForm = ":is(:scope :not([form], :scope form *))";
if (form.id) {
scope = document;
ownedByForm = `:is([form='${form.id}'],:is(#${form.id} :not([form], #${form.id} form *))`;
}

// 4. Let entry list be a new empty entry list.
let entryList: [string, string | File][] = [];

// 5. For each element field in controls, in tree order:
// 1. If any of the following is true:
// - The field element has a datalist element ancestor.
// - The field element is disabled.
// - The field element is a button but it is not submitter.
// - The field element is an input element whose type attribute is in the Checkbox state and whose checkedness is false.
// - The field element is an input element whose type attribute is in the Radio Button state and whose checkedness is false.
// Then continue.
//
// IMPLEMENTATION NOTES:
// - We skip the datalist check since 1. browsers don't do it and 2. the only valid child of a datalist
// is an option, which is not a submittable element.
// - The "button is not submitter" logic is handled at the start of the for loop
let disabled = ":disabled";
let uncheckedCheckboxOrRadio = "input[type~='radio checkbox']:not([checked])";
let unNamedExcludingImageButtons =
":is([name=''],:not([name]):not(input[type=image])";
let disqualifiers = [
disabled,
uncheckedCheckboxOrRadio,
unNamedExcludingImageButtons,
].join(",");

let selector = `${submittable}${ownedByForm}:not(${disqualifiers})`;
let controls = scope.querySelectorAll<SubmittableElement>(selector);

for (let field of controls) {
// IMPLEMENTATION NOTES:
// - "button" is defined in the spec as any element with the prose "The element is a button"
// See: https://html.spec.whatwg.org/multipage/input.html
// and https://html.spec.whatwg.org/multipage/form-elements.html
let isButton =
isButtonElement(field) ||
(isInputElement(field) &&
["submit", "image", "button", "reset"].includes(field.type));
if (isButton && field !== submitter) continue;

// 2. If the field element is an input element whose type attribute is in the Image Button state, then:
// 1. If the field element has a name attribute specified and its value is not the empty string, let name be that value followed by a single U+002E FULL STOP character (.). Otherwise, let name be the empty string.
// 2. Let namex be the string consisting of the concatenation of name and a single U+0078 LATIN SMALL LETTER X character (x).
// 3. Let namey be the string consisting of the concatenation of name and a single U+0079 LATIN SMALL LETTER Y character (y).
// 4. The field element is submitter, and before this algorithm was invoked the user indicated a coordinate. Let x be the x-component of the coordinate selected by the user, and let y be the y-component of the coordinate selected by the user.
// 5. Create an entry with namex and x, and append it to entry list.
// 6. Create an entry with namey and y, and append it to entry list.
// 7. Continue.
if (
isImageButtonElement(field) &&
field === submitter &&
field[SELECTED_COORDINATE]
) {
let prefix = field.name ? `${field.name}.` : "";
let { x, y } = field[SELECTED_COORDINATE];
entryList.push([`${prefix}x`, String(x)]);
entryList.push([`${prefix}y`, String(y)]);
continue;
}

// 3. If the field is a form-associated custom element, then perform the entry construction algorithm given field and entry list, then continue.
// 4. If either the field element does not have a name attribute specified, or its name attribute's value is the empty string, then continue.
// 5. Let name be the value of the field element's name attribute.
let { name } = field;
if (!name) continue;

// 6. If the field element is a select element, then for each option element in the select element's list of options whose selectedness is true and that is not disabled, create an entry with name and the value of the option element, and append it to entry list.
if (isSelectElement(field)) {
for (let option of field.selectedOptions) {
if (!option.disabled) entryList.push([name, option.value]);
}
}
// 7. Otherwise, if the field element is an input element whose type attribute is in the Checkbox state or the Radio Button state, then:
// 1. If the field element has a value attribute specified, then let value be the value of that attribute; otherwise, let value be the string "on".
// 2. Create an entry with name and value, and append it to entry list.
else if (
isInputElement(field) &&
["radio", "checkbox"].includes(field.type)
) {
if (field.checked) entryList.push([name, field.value]);
}
// 8. Otherwise, if the field element is an input element whose type attribute is in the File Upload state, then:
// 1. If there are no selected files, then create an entry with name and a new File object with an empty name, application/octet-stream as type, and an empty body, and append it to entry list.
// 2. Otherwise, for each file in selected files, create an entry with name and a File object representing the file, and append it to entry list.
else if (isInputElement(field) && field.type === "file") {
if (field.files?.length) {
for (let file of field.files) {
entryList.push([name, file]);
}
} else {
entryList.push([name, ""]);
}
}
// 9. Otherwise, if the field element is an input element whose type attribute is in the Hidden state and name is an ASCII case-insensitive match for "_charset_":
// 1. Let charset be the name of encoding if encoding is given, and "UTF-8" otherwise.
// 2. Create an entry with name and charset, and append it to entry list.

// 10. Otherwise, create an entry with name and the value of the field element, and append it to entry list.
else {
entryList.push([name, field.value]);
}

// 11. If the element has a dirname attribute, and that attribute's value is not the empty string, then:
// 1. Let dirname be the value of the element's dirname attribute.
// 2. Let dir be the string "ltr" if the directionality of the element is 'ltr', and "rtl" otherwise (i.e., when the directionality of the element is 'rtl').
// 3. Create an entry with dirname and dir, and append it to entry list.
if (isInputElement(field) || isTextareaElement(field)) {
if (field.dirName) {
entryList.push([
field.dirName,
getComputedStyle(field).getPropertyValue("direction"),
]);
}
}
}
// 6. Let form data be a new FormData object associated with entry list.
// 7. Fire an event named formdata at form using FormDataEvent, with the formData attribute initialized to form data and the bubbles attribute initialized to true.
// 8. Set form's constructing entry list to false.
// 9. Return a clone of entry list.
//
// IMPLEMENTATION NOTES:
// - We return FormData rather than an entry list
// - We don't fire the formdata event; the browser will do that for us, though unfortunately it's before we've populated it :-/
let formData = new FormData();
for (let [name, value] of entryList) formData.append(name, value);
return formData;
}

function setNextNavigationSubmission(submission: Submission) {
nextNavigationSubmission = submission;
}
Expand All @@ -1324,6 +1495,18 @@ 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";
}

function isSelectElement(object: any): object is HTMLSelectElement {
return isHtmlElement(object) && object.tagName.toLowerCase() === "select";
}

function isTextareaElement(object: any): object is HTMLTextAreaElement {
return isHtmlElement(object) && object.tagName.toLowerCase() === "textarea";
}

/**
* 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 7b72226

Please sign in to comment.