Bug 171589

Actually, the webkit DOMFormData.idl is a bit out of date with the spec. Looks like the following is missing in addition to the Exposed(Worker): void delete(USVString name); FormDataEntryValue? get(USVString name); sequence<FormDataEntryValue> getAll(USVString name); boolean has(USVString name); void set(USVString name, USVString value); void set(USVString name, Blob blobValue, optional USVString filename); iterable<USVString, FormDataEntryValue>; Let me see if I can just bring the impl update to spec here.

Looks like this was implemented in https://bugs.webkit.org/show_bug.cgi?id=176674 (worker) and https://bugs.webkit.org/show_bug.cgi?id=176659 (the rest).

Created attachment 410122 [details] Patch

Created attachment 410125 [details] Patch

Created attachment 410126 [details] Patch

Created attachment 410131 [details] Patch

Is the optional argument not compiling?

(In reply to Sam Weinig from comment #7) > Is the optional argument not compiling? Right, I got a "not supported" error from the IDL processing when I tried.

Comment on attachment 410131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=410131&action=review > Source/WebCore/html/DOMFormData.idl:40 > + // FIXME: should be optional HTMLFormElement. I think we should make this work. This change makes our IDL look worse. I am actually very surprised it does not work.

(In reply to Rob Buis from comment #8) > (In reply to Sam Weinig from comment #7) > > Is the optional argument not compiling? > > Right, I got a "not supported" error from the IDL processing when I tried. hm, interesting. If you can try again, and give me the full output, I can probably point to what's wrong.

(In reply to Sam Weinig from comment #10) > (In reply to Rob Buis from comment #8) > > (In reply to Sam Weinig from comment #7) > > > Is the optional argument not compiling? > > > > Right, I got a "not supported" error from the IDL processing when I tried. > > hm, interesting. If you can try again, and give me the full output, I can > probably point to what's wrong. Optional arguments of non-nullable wrapper types are not supported () at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 118. $VAR1 = ' at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 6196. CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70), IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8), "create", " ") called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 7765 CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70), "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 7696 CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70), "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", IDLInterface=HASH(0x7f96e289ffa8)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 4498 CodeGeneratorJS::GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118), IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500), ARRAY(0x7f96e28a0ff8)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm line 177 CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118), IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500), ARRAY(0x7f96e28a0ff8)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGenerator.pm line 256 CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0), IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/CodeGenerator.pm line 177 CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0), IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"...) called at WebCore/bindings/scripts/generate-bindings.pl line 132 main::generateBindings("WebCore/html/DOMFormData.idl") called at WebCore/bindings/scripts/generate-bindings.pl line 82 '; make[1]: *** [JSDOMFormData.h] Error 255 Command /bin/sh failed with exit code 2 ** BUILD FAILED **

(In reply to Chris Dumez from comment #11) > (In reply to Sam Weinig from comment #10) > > (In reply to Rob Buis from comment #8) > > > (In reply to Sam Weinig from comment #7) > > > > Is the optional argument not compiling? > > > > > > Right, I got a "not supported" error from the IDL processing when I tried. > > > > hm, interesting. If you can try again, and give me the full output, I can > > probably point to what's wrong. > > Optional arguments of non-nullable wrapper types are not supported () at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGeneratorJS.pm line 118. > $VAR1 = ' at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGeneratorJS.pm line 6196. > CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70), > IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8), > "create", " ") called at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGeneratorJS.pm line 7765 > CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70), > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", > IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400)) > called at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGeneratorJS.pm line 7696 > CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70), > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", > IDLInterface=HASH(0x7f96e289ffa8)) called at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGeneratorJS.pm line 4498 > CodeGeneratorJS:: > GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118), > IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500), > ARRAY(0x7f96e28a0ff8)) called at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGeneratorJS.pm line 177 > CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118), > IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS > ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500), > ARRAY(0x7f96e28a0ff8)) called at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGenerator.pm line 256 > CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0), > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY > ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > CodeGenerator.pm line 177 > CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0), > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY > ENABLE_ACCESSIBILIT"...) called at > WebCore/bindings/scripts/generate-bindings.pl line 132 > main::generateBindings("WebCore/html/DOMFormData.idl") called at > WebCore/bindings/scripts/generate-bindings.pl line 82 > '; > make[1]: *** [JSDOMFormData.h] Error 255 > Command /bin/sh failed with exit code 2 > ** BUILD FAILED ** Oh fun. Wonder why I (or perhaps someone else) did that. Probably about disambiguating between null and optional, but I don't think that should matter here. Will take a look later on this afternoon.

(In reply to Sam Weinig from comment #12) > (In reply to Chris Dumez from comment #11) > > (In reply to Sam Weinig from comment #10) > > > (In reply to Rob Buis from comment #8) > > > > (In reply to Sam Weinig from comment #7) > > > > > Is the optional argument not compiling? > > > > > > > > Right, I got a "not supported" error from the IDL processing when I tried. > > > > > > hm, interesting. If you can try again, and give me the full output, I can > > > probably point to what's wrong. > > > > Optional arguments of non-nullable wrapper types are not supported () at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGeneratorJS.pm line 118. > > $VAR1 = ' at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGeneratorJS.pm line 6196. > > CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70), > > IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8), > > "create", " ") called at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGeneratorJS.pm line 7765 > > CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70), > > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", > > IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400)) > > called at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGeneratorJS.pm line 7696 > > CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70), > > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", > > IDLInterface=HASH(0x7f96e289ffa8)) called at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGeneratorJS.pm line 4498 > > CodeGeneratorJS:: > > GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118), > > IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500), > > ARRAY(0x7f96e28a0ff8)) called at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGeneratorJS.pm line 177 > > CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118), > > IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS > > ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500), > > ARRAY(0x7f96e28a0ff8)) called at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGenerator.pm line 256 > > CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0), > > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY > > ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > CodeGenerator.pm line 177 > > CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0), > > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY > > ENABLE_ACCESSIBILIT"...) called at > > WebCore/bindings/scripts/generate-bindings.pl line 132 > > main::generateBindings("WebCore/html/DOMFormData.idl") called at > > WebCore/bindings/scripts/generate-bindings.pl line 82 > > '; > > make[1]: *** [JSDOMFormData.h] Error 255 > > Command /bin/sh failed with exit code 2 > > ** BUILD FAILED ** > > Oh fun. Wonder why I (or perhaps someone else) did that. Probably about > disambiguating between null and optional, but I don't think that should > matter here. Will take a look later on this afternoon. I think the issue is that our generated bindings always call the implementation function with ALL parameters (even the optional ones that were omitted). As a result, we need a value to pass the implementation. When a parameter is an interface and nullable, we pass null to the implementation. However, when the interface is not nullable, the bindings currently don't know what to pass. I guess there could be at least 2 ways to fix it: 1. Pass null to the implementation when the parameter is omitted and it is an interface type (even if not nullable). We'd have to make sure we're properly throwing when the JS passes null though. 2. Omit the parameters that were omitted in JS when calling the implementation function

(In reply to Chris Dumez from comment #13) > (In reply to Sam Weinig from comment #12) > > (In reply to Chris Dumez from comment #11) > > > (In reply to Sam Weinig from comment #10) > > > > (In reply to Rob Buis from comment #8) > > > > > (In reply to Sam Weinig from comment #7) > > > > > > Is the optional argument not compiling? > > > > > > > > > > Right, I got a "not supported" error from the IDL processing when I tried. > > > > > > > > hm, interesting. If you can try again, and give me the full output, I can > > > > probably point to what's wrong. > > > > > > Optional arguments of non-nullable wrapper types are not supported () at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGeneratorJS.pm line 118. > > > $VAR1 = ' at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGeneratorJS.pm line 6196. > > > CodeGeneratorJS::GenerateParametersCheck(ARRAY(0x7f96c682fa70), > > > IDLOperation=HASH(0x7f96e28a1400), IDLInterface=HASH(0x7f96e289ffa8), > > > "create", " ") called at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGeneratorJS.pm line 7765 > > > CodeGeneratorJS::GenerateConstructorDefinition(ARRAY(0x7f96c682fa70), > > > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", > > > IDLInterface=HASH(0x7f96e289ffa8), undef, IDLOperation=HASH(0x7f96e28a1400)) > > > called at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGeneratorJS.pm line 7696 > > > CodeGeneratorJS::GenerateConstructorDefinitions(ARRAY(0x7f96c682fa70), > > > "JSDOMFormData", "JSDOMFormDataPrototype", "FormData", > > > IDLInterface=HASH(0x7f96e289ffa8)) called at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGeneratorJS.pm line 4498 > > > CodeGeneratorJS:: > > > GenerateImplementation(CodeGeneratorJS=HASH(0x7f96e28a1118), > > > IDLInterface=HASH(0x7f96e289ffa8), ARRAY(0x7f96c6850500), > > > ARRAY(0x7f96e28a0ff8)) called at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGeneratorJS.pm line 177 > > > CodeGeneratorJS::GenerateInterface(CodeGeneratorJS=HASH(0x7f96e28a1118), > > > IDLInterface=HASH(0x7f96e289ffa8), "ENABLE_3D_TRANSFORMS > > > ENABLE_ACCESSIBILITY ENABLE_ACCESSIBILIT"..., ARRAY(0x7f96c6850500), > > > ARRAY(0x7f96e28a0ff8)) called at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGenerator.pm line 256 > > > CodeGenerator::ProcessInterfaces(CodeGenerator=HASH(0x7f96e28a4cf0), > > > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY > > > ENABLE_ACCESSIBILIT"..., CodeGeneratorJS=HASH(0x7f96e28a1118)) called at > > > /Volumes/Work/WebKit/OpenSource/Source/WebCore/bindings/scripts/ > > > CodeGenerator.pm line 177 > > > CodeGenerator::ProcessDocument(CodeGenerator=HASH(0x7f96e28a4cf0), > > > IDLDocument=HASH(0x7f96e28a10a0), "ENABLE_3D_TRANSFORMS ENABLE_ACCESSIBILITY > > > ENABLE_ACCESSIBILIT"...) called at > > > WebCore/bindings/scripts/generate-bindings.pl line 132 > > > main::generateBindings("WebCore/html/DOMFormData.idl") called at > > > WebCore/bindings/scripts/generate-bindings.pl line 82 > > > '; > > > make[1]: *** [JSDOMFormData.h] Error 255 > > > Command /bin/sh failed with exit code 2 > > > ** BUILD FAILED ** > > > > Oh fun. Wonder why I (or perhaps someone else) did that. Probably about > > disambiguating between null and optional, but I don't think that should > > matter here. Will take a look later on this afternoon. > > I think the issue is that our generated bindings always call the > implementation function with ALL parameters (even the optional ones that > were omitted). As a result, we need a value to pass the implementation. When > a parameter is an interface and nullable, we pass null to the > implementation. However, when the interface is not nullable, the bindings > currently don't know what to pass. > > I guess there could be at least 2 ways to fix it: > 1. Pass null to the implementation when the parameter is omitted and it is > an interface type (even if not nullable). We'd have to make sure we're > properly throwing when the JS passes null though. > 2. Omit the parameters that were omitted in JS when calling the > implementation function #1 is what we will likely want to do. For non-wrapper types, what we would do here is have the implementation use Optional<type> as the parameter. This does lead to the potential for a problem if you have a case like: undefined foo(optional long a); undefined foo(long? a); We wouldn't be able to distinguish between the two overloads in the implementation (I am not sure that is actually valid WebIDL, but you get the idea) as they would both be: void foo(Optional<int>); For wrapper types, we don't use Optional<>, but use nullable pointers for nullable types, and references for non-nullable types. So if you have: undefined foo(optional Node a); undefined foo(Node? a); you would again would have the same situation, where both the implementation functions would have the same signature: void foo(Node*). But, given we already live with the ambiguity with the non-wrapper type case, I am not sure why we didn't implement the wrapper case the same way (but using pointers instead of Optional). Seems like we should just treat optional Node and Node? the same at the implementation level (but of course keep the bindings throwing for the optional case when you pass null). One day, we may have to make things more explicit, and have the implementations use specific IDLOptional<> / IDLNullable<> types, but lets wait for that to actually crop up.

Ok, blocking bug is now in. You should be able to use: constructor(optional HTMLFormElement form);

(In reply to Sam Weinig from comment #15) > Ok, blocking bug is now in. You should be able to use: > > constructor(optional HTMLFormElement form); Yay, thanks Sam.

Created attachment 410303 [details] Patch

(In reply to Chris Dumez from comment #16) > (In reply to Sam Weinig from comment #15) > > Ok, blocking bug is now in. You should be able to use: > > > > constructor(optional HTMLFormElement form); > > Yay, thanks Sam. Yes, thanks Sam! The patch is now trivial.

Committed r267866: <https://trac.webkit.org/changeset/267866> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410303 [details].

<rdar://problem/69870343>