ElementInternals.setFormValue(<nullish value>) doesn't clear submissi…

…on value

https://bugs.webkit.org/show_bug.cgi?id=258870
<rdar://problem/111802198>

Reviewed by Ryosuke Niwa.

With current bindings implementation, given an optional nullish interface / union type without default value,
it's impossible to distiungish in DOM code whether a null / undefined was passed or an argument was missing:
both compile to std::nullopt.

This was causing two bugs in ElementInternals.setFormValue() [1]:
  1) nullish `value` parameter was not clearing the submission value;
  2) nullish `state` parameter was perceived as missing and the `value` was used instead.

On one hand, we could revise all WebIDL files with optional nullish interface / union types to ensure that
we don't have extra / missing default values (oftentimes we do), and then make ones without defaults values
compile to std::nullopt (for missing argument) / std::nullptr_t (for nullish argument).

On the other, that would be quite massive change and this distinction isn't current needed anywhere but
ElementInternals.setFormValue(), so this change hand-rolls custom bindings just for this method, fixing
both issues by checking argument count and leveraging std::nullptr_t.

[1] https://html.spec.whatwg.org/multipage/custom-elements.html#dom-elementinternals-setformvalue

* LayoutTests/fast/forms/state-restore-form-associated-custom-elements-2-expected.txt: Added.
* LayoutTests/fast/forms/state-restore-form-associated-custom-elements-2.html: Added.
* LayoutTests/fast/forms/state-restore-form-associated-custom-elements.html:
* LayoutTests/imported/w3c/web-platform-tests/custom-elements/form-associated/ElementInternals-setFormValue-nullish-value-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/custom-elements/form-associated/ElementInternals-setFormValue-nullish-value.html: Added.
* Source/WebCore/bindings/js/JSCustomElementInterface.cpp:
(WebCore::JSCustomElementInterface::invokeFormStateRestoreCallback):
* Source/WebCore/bindings/js/JSElementInternalsCustom.cpp:
(WebCore::JSElementInternals::setFormValue):
* Source/WebCore/dom/ElementInternals.cpp:
(WebCore::ElementInternals::setFormValue):
* Source/WebCore/dom/ElementInternals.h:
* Source/WebCore/dom/ElementInternals.idl:
* Source/WebCore/html/CustomElementFormValue.h:
* Source/WebCore/html/FormAssociatedCustomElement.cpp:
(WebCore::FormAssociatedCustomElement::setFormValue):
(WebCore::FormAssociatedCustomElement::appendFormData):
(WebCore::FormAssociatedCustomElement::saveFormControlState const):
* Source/WebCore/html/FormAssociatedCustomElement.h:

Canonical link: https://commits.webkit.org/266126@main

LayoutTests/fast/forms/state-restore-form-associated-custom-elements-2-expected.txt

@@ -0,0 +1,29 @@
+
+Upgraded form-associated custom elements without an owner:
+PASS $("noowner-upgrade1").restoredState is undefined
+PASS $("noowner-upgrade2").restoredState is "bar"
+PASS $("noowner-upgrade3").restoredState is undefined
+PASS isFormDataEqual($("noowner-upgrade4").restoredState, __formData1) is true
+
+Upgraded form-associated custom elements with a form owner:
+PASS $("upgrade1").restoredState is undefined
+PASS $("upgrade2").restoredState is undefined
+PASS isFormDataEqual($("upgrade3").restoredState, __formData1) is true
+PASS $("upgrade4").restoredState is "bar"
+
+Predefined form-associated custom elements without an owner:
+PASS $("noowner-predefined1").restoredState is undefined
+PASS $("noowner-predefined2").restoredState is "bar"
+PASS $("noowner-predefined3").restoredState is undefined
+PASS isFormDataEqual($("noowner-predefined4").restoredState, __formData2) is true
+
+Predefined form-associated custom elements with a form owner:
+PASS $("predefined1").restoredState is undefined
+PASS isFormDataEqual($("predefined2").restoredState, __formData1) is true
+PASS $("predefined3").restoredState is undefined
+PASS $("predefined4").restoredState is "foo"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+

LayoutTests/fast/forms/state-restore-form-associated-custom-elements-2.html

@@ -0,0 +1,145 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="resources/common.js"></script>
+</head>
+<body>
+<div id="console"></div>
+
+<input id=emptyOnFirstVisit>
+
+<script>
+jsTestIsAsync = true;
+
+window.__formData1 = new FormData;
+__formData1.append("foo", "bar");
+
+window.__formData2 = new FormData;
+__formData2.append("a", "1");
+__formData2.append("b", "2");
+
+function getXFooConstructor() {
+    return class extends HTMLElement {
+        static formAssociated = true;
+
+        constructor() {
+            super();
+            this._internals = this.attachInternals();
+        }
+
+        connectedCallback() {
+            const value = this._parseFormValue(this.dataset.submissionValue);
+            const state = this._parseFormValue(this.dataset.state);
+
+            this._internals.setFormValue(value, state);
+        }
+
+        formStateRestoreCallback(state) {
+            this.restoredState = state;
+        }
+
+        _parseFormValue(attrValue) {
+            return typeof attrValue === "string" && attrValue.startsWith("@") ? window[`__${attrValue.slice(1)}`] : attrValue;
+        }
+    };
+}
+
+function isFormDataEqual(a, b) {
+    const bEntries = [...b];
+
+    return [...a].every(([aKey, aValue], index) => {
+        const [bKey, bValue] = bEntries[index];
+        return aKey === bKey && aValue === bValue;
+    });
+}
+
+customElements.define("x-foo-predefined", getXFooConstructor());
+
+function makeForms(stage) {
+    var beforeForms = '<div id=parent>' +
+        '<x-foo-upgrade id="noowner-upgrade2" name="noowner-upgrade2" data-submission-value="@formData2" data-state="bar"></x-foo-upgrade>' +
+        '<x-foo-upgrade id="noowner-upgrade4" name="noowner-upgrade4" data-submission-value="foo" data-state="@formData1"></x-foo-upgrade>' +
+        '<x-foo-predefined id="noowner-predefined2" name="noowner-predefined2" data-submission-value="foo" data-state="bar"></x-foo-predefined>' +
+        '<x-foo-predefined id="noowner-predefined4" name="noowner-predefined4" data-submission-value="@formData1" data-state="@formData2"></x-foo-predefined>';
+
+    var backForm = '<form action="data:text/html,&lt;script>history.back();&lt;/script>" id=form1 name=form1>' +
+        '</form>';
+
+    var query = stage == 1 ? "?session=0123456" : "?session=7654321111";
+    var sameActionForm1 = '<form action="http://example.com/foo.cgi' + query + '#bar" id=form2 name=form2>' +
+        '<x-foo-upgrade id="upgrade2" name="upgrade2" data-submission-value="foo"></x-foo-upgrade>' +
+        '<x-foo-upgrade id="upgrade4" name="upgrade4" data-submission-value="foo" data-state="bar"></x-foo-upgrade>' +
+        '<x-foo-predefined id="predefined2" name="predefined2" data-submission-value="foo" data-state="@formData1"></x-foo-predefined>' +
+        '<x-foo-predefined id="predefined4" name="predefined4" data-submission-value="@formData2" data-state="foo"></x-foo-predefined>' +
+        '</form>';
+
+    var sameActionForm2 = '<form action="http://example.com/foo.cgi?action=login#bar" id=form3 name=form3>' +
+        '<x-foo-upgrade id="upgrade1" name="upgrade1" data-submission-value="@formData2"></x-foo-upgrade>' +
+        '<x-foo-upgrade id="upgrade3" name="upgrade3" data-submission-value="@formData2" data-state="@formData1"></x-foo-upgrade>' +
+        '<x-foo-predefined id="predefined1" name="predefined1" data-submission-value="foo"></x-foo-predefined>' +
+        '<x-foo-predefined id="predefined3" name="predefined3" data-submission-value="bar"></x-foo-predefined>' +
+        '</form>';
+
+    var afterForms =
+        '<x-foo-upgrade id="noowner-upgrade1" name="noowner-upgrade1" data-submission-value="foo"></x-foo-upgrade>' +
+        '<x-foo-upgrade id="noowner-upgrade3" name="noowner-upgrade3" data-submission-value="@formData2"></x-foo-upgrade>' +
+        '<x-foo-predefined id="noowner-predefined1" name="noowner-predefined1" data-submission-value="foo"></x-foo-predefined>' +
+        '<x-foo-predefined id="noowner-predefined3" name="noowner-predefined3" data-submission-value="@formData1"></x-foo-predefined>' +
+        '</div>';
+
+    document.write(
+        beforeForms +
+        (stage === 1 ? backForm + sameActionForm1 : sameActionForm1 + backForm) +
+        sameActionForm2 +
+        afterForms
+    );
+
+    customElements.define("x-foo-upgrade", getXFooConstructor());
+}
+
+function runTest()
+{
+    var state = $('emptyOnFirstVisit');
+    if (!state.value) {
+        state.value = 'visited';
+        makeForms(1);
+
+        setTimeout(function() {
+            $('form1').submit();
+        });
+    } else {
+        makeForms(2);
+
+        debug('\nUpgraded form-associated custom elements without an owner:');
+        shouldBe('$("noowner-upgrade1").restoredState', 'undefined');
+        shouldBeEqualToString('$("noowner-upgrade2").restoredState', 'bar');
+        shouldBe('$("noowner-upgrade3").restoredState', 'undefined');
+        shouldBeTrue('isFormDataEqual($("noowner-upgrade4").restoredState, __formData1)');
+
+        debug('\nUpgraded form-associated custom elements with a form owner:');
+        shouldBe('$("upgrade1").restoredState', 'undefined');
+        shouldBe('$("upgrade2").restoredState', 'undefined');
+        shouldBeTrue('isFormDataEqual($("upgrade3").restoredState, __formData1)');
+        shouldBeEqualToString('$("upgrade4").restoredState', 'bar');
+
+        debug('\nPredefined form-associated custom elements without an owner:');
+        shouldBe('$("noowner-predefined1").restoredState', 'undefined');
+        shouldBeEqualToString('$("noowner-predefined2").restoredState', 'bar');
+        shouldBe('$("noowner-predefined3").restoredState', 'undefined');
+        shouldBeTrue('isFormDataEqual($("noowner-predefined4").restoredState, __formData2)');
+
+        debug('\nPredefined form-associated custom elements with a form owner:');
+        shouldBe('$("predefined1").restoredState', 'undefined');
+        shouldBeTrue('isFormDataEqual($("predefined2").restoredState, __formData1)');
+        shouldBe('$("predefined3").restoredState', 'undefined');
+        shouldBeEqualToString('$("predefined4").restoredState', 'foo');
+
+        $('parent').innerHTML = '';
+        setTimeout(function() { finishJSTest(); });
+    }
+}
+
+runTest();
+</script>
+</body>

LayoutTests/fast/forms/state-restore-form-associated-custom-elements.html

@@ -29,10 +29,13 @@
         }
 
         connectedCallback() {
-            this._internals.setFormValue(
-                this._parseFormValue(this.dataset.submissionValue),
-                this._parseFormValue(this.dataset.state),
-            );
+            const value = this._parseFormValue(this.dataset.submissionValue);
+            const state = this._parseFormValue(this.dataset.state);
+
+            if (state == null)
+                this._internals.setFormValue(value);
+            else
+                this._internals.setFormValue(value, state);
         }
 
         formStateRestoreCallback(state) {

LayoutTests/imported/w3c/web-platform-tests/custom-elements/form-associated/ElementInternals-setFormValue-nullish-value-expected.txt

@@ -0,0 +1,4 @@
+
+PASS ElementInternals.setFormValue(null) clears submission value
+PASS ElementInternals.setFormValue(undefined) clears submission value
+

LayoutTests/imported/w3c/web-platform-tests/custom-elements/form-associated/ElementInternals-setFormValue-nullish-value.html

@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>ElementInternals.setFormValue(nullish value) should clear submission value</title>
+    <link rel="help" href="https://html.spec.whatwg.org/multipage/custom-elements.html#dom-elementinternals-setformvalue">
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+    <script>
+        customElements.define("test-form-element", class extends HTMLElement {
+            static formAssociated = true;
+            constructor() {
+                super();
+                this.internals = this.attachInternals();
+            }
+        });
+    </script>
+</head>
+<body>
+    <form id="form-null">
+        <test-form-element id="input-null" name="input-null"></test-form-element>
+    </form>
+
+    <form id="form-undefined">
+        <test-form-element id="input-undefined" name="input-undefined"></test-form-element>
+    </form>
+
+    <script>
+      test(() => {
+        const input = document.getElementById("input-null");
+        input.internals.setFormValue("fail");
+        input.internals.setFormValue(null);
+        const formData = new FormData(document.getElementById("form-null"));
+        assert_false(formData.has("input-null"));
+      }, "ElementInternals.setFormValue(null) clears submission value");
+
+      test(() => {
+        const input = document.getElementById("input-undefined");
+        input.internals.setFormValue("fail");
+        input.internals.setFormValue(undefined);
+        const formData = new FormData(document.getElementById("form-undefined"));
+        assert_false(formData.has("input-undefined"));
+      }, "ElementInternals.setFormValue(undefined) clears submission value");
+    </script>
+</body>
+</html>

Source/WebCore/bindings/js/JSCustomElementInterface.cpp

@@ -391,6 +391,8 @@ void JSCustomElementInterface::invokeFormStateRestoreCallback(Element& element,
             args.append(jsString(vm, state));
         }, [&](RefPtr<File>) {
             ASSERT_NOT_REACHED();
+        }, [](std::nullptr_t) {
+            ASSERT_NOT_REACHED();
         });
 
         args.append(jsNontrivialString(vm, "restore"_s));

Source/WebCore/bindings/js/JSElementInternalsCustom.cpp

@@ -31,13 +31,46 @@
 #include "JSDOMConvertInterface.h"
 #include "JSDOMConvertNullable.h"
 #include "JSDOMConvertSequences.h"
+#include "JSDOMFormData.h"
 #include "JSElement.h"
+#include "JSFile.h"
 #include "WebCoreJSClientData.h"
 #include <JavaScriptCore/ObjectConstructor.h>
 
 namespace WebCore {
 using namespace JSC;
 
+JSValue JSElementInternals::setFormValue(JSGlobalObject& lexicalGlobalObject, CallFrame& callFrame)
+{
+    using JSCustomElementFormValue = IDLUnion<IDLNull, IDLInterface<File>, IDLUSVString, IDLInterface<DOMFormData>>;
+
+    auto& vm = lexicalGlobalObject.vm();
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    if (UNLIKELY(callFrame.argumentCount() < 1)) {
+        throwException(&lexicalGlobalObject, throwScope, createNotEnoughArgumentsError(&lexicalGlobalObject));
+        return { };
+    }
+
+    EnsureStillAliveScope argument0 = callFrame.uncheckedArgument(0);
+    auto value = convert<JSCustomElementFormValue>(lexicalGlobalObject, argument0.value());
+    RETURN_IF_EXCEPTION(throwScope, { });
+
+    std::optional<CustomElementFormValue> state;
+    if (callFrame.argumentCount() > 1) {
+        EnsureStillAliveScope argument1 = callFrame.argument(1);
+        state = convert<JSCustomElementFormValue>(lexicalGlobalObject, argument1.value());
+        RETURN_IF_EXCEPTION(throwScope, { });
+    }
+
+    auto result = wrapped().setFormValue(WTFMove(value), WTFMove(state));
+    if (UNLIKELY(result.hasException())) {
+        propagateException(lexicalGlobalObject, throwScope, result.releaseException());
+        return { };
+    }
+
+    return jsUndefined();
+}
+
 static JSValue getElementsArrayAttribute(JSGlobalObject& lexicalGlobalObject, const JSElementInternals& thisObject, const QualifiedName& attributeName)
 {
     auto& vm = JSC::getVM(&lexicalGlobalObject);

Source/WebCore/dom/ElementInternals.cpp

@@ -61,7 +61,7 @@ ExceptionOr<RefPtr<HTMLFormElement>> ElementInternals::form() const
     return Exception { NotSupportedError };
 }
 
-ExceptionOr<void> ElementInternals::setFormValue(std::optional<CustomElementFormValue>&& value, std::optional<CustomElementFormValue>&& state)
+ExceptionOr<void> ElementInternals::setFormValue(CustomElementFormValue&& value, std::optional<CustomElementFormValue>&& state)
 {
     if (RefPtr element = elementAsFormAssociatedCustom()) {
         element->setFormValue(WTFMove(value), WTFMove(state));

Source/WebCore/dom/ElementInternals.h

@@ -51,7 +51,7 @@ class ElementInternals final : public ScriptWrappable, public RefCounted<Element
 
     ExceptionOr<RefPtr<HTMLFormElement>> form() const;
 
-    ExceptionOr<void> setFormValue(std::optional<CustomElementFormValue>&&, std::optional<CustomElementFormValue>&& = std::nullopt);
+    ExceptionOr<void> setFormValue(CustomElementFormValue&&, std::optional<CustomElementFormValue>&& state);
 
     ExceptionOr<void> setValidity(ValidityStateFlags, String&& message, HTMLElement* validationAnchor);
     ExceptionOr<bool> willValidate() const;

Source/WebCore/dom/ElementInternals.idl

@@ -23,8 +23,6 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-typedef (File or USVString or DOMFormData) FormValue;
-
 [
     GenerateIsReachable=ImplElementRoot,
     GenerateAddOpaqueRoot=element,
@@ -33,7 +31,7 @@ typedef (File or USVString or DOMFormData) FormValue;
 ] interface ElementInternals {
     readonly attribute ShadowRoot? shadowRoot;
 
-    undefined setFormValue(FormValue? value, optional FormValue? state);
+    [Custom] undefined setFormValue(any value, optional any state);
 
     readonly attribute HTMLFormElement? form;
 

Source/WebCore/html/CustomElementFormValue.h

@@ -31,6 +31,6 @@
 
 namespace WebCore {
 
-using CustomElementFormValue = std::variant<RefPtr<File>, String, RefPtr<DOMFormData>>;
+using CustomElementFormValue = std::variant<std::nullptr_t, RefPtr<File>, String, RefPtr<DOMFormData>>;
 
 } // namespace WebCore