Bug 215837

Summary: Implement DataTransfer constructor and multipart form filename encoding as other browsers do
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, clopez, esprehn+autocc, ews-watchlist, kangil.han, kondapallykalyan, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Alex Christensen 2020-08-25 20:59:40 PDT
Implement DataTransfer constructor and multipart form filename encoding as other browsers do
Comment 1 Alex Christensen 2020-08-25 21:01:09 PDT
Created attachment 407270 [details]
Patch
Comment 2 Alex Christensen 2020-08-25 21:07:39 PDT
Created attachment 407271 [details]
Patch
Comment 3 youenn fablet 2020-08-26 05:37:01 PDT
imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/submit-file.sub.html needs to be updated.
Comment 4 Alex Christensen 2020-08-26 09:51:35 PDT
Created attachment 407306 [details]
Patch
Comment 5 EWS Watchlist 2020-08-26 09:52:34 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 6 youenn fablet 2020-08-26 11:11:22 PDT
Comment on attachment 407306 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407306&action=review

> Source/WebCore/dom/DataTransfer.cpp:97
> +    dataTransfer->m_effectAllowed = "none";

It is a bit sad to set it to none while we are initialising it to uninitialised.
Can we change DataTransfer constructor?

> Source/WebCore/platform/network/FormDataBuilder.cpp:179
> +    appendQuoted(buffer, encoding.encode(filename, UnencodableHandling::Entities));

Do we have test coverage?

> LayoutTests/imported/w3c/web-platform-tests/html/editing/dnd/datastore/datatransfer-types-expected.txt:3
> +FAIL Relationship between types and items assert_equals: expected ["text/plain"] but got ["text/plain"]

This error seems strange, we got what we expect so why are we failing?

> LayoutTests/imported/w3c/web-platform-tests/html/semantics/forms/form-submission-0/submit-file.sub.html:7
> +<form id=testform method=post action="/html/semantics/forms/form-submission-0/resources/file-submission.py" target=testframe enctype="multipart/form-data">

Might be better to use http://{{hosts[alt][]}}:{{ports[http][0]}}
That way we keep cross origin posting which might be useful for the test.

We will need to upload this change to WPT as well.
If you use webkit-patch upload, it might allow you to automate the process and I can merge the changes in WPT.
Comment 7 Alex Christensen 2020-08-26 11:35:01 PDT
(In reply to youenn fablet from comment #6)
> > Source/WebCore/platform/network/FormDataBuilder.cpp:179
> > +    appendQuoted(buffer, encoding.encode(filename, UnencodableHandling::Entities));
> 
> Do we have test coverage?
Yes.  This is what makes the send-file-form-{encoding}.html tests pass.

> > LayoutTests/imported/w3c/web-platform-tests/html/editing/dnd/datastore/datatransfer-types-expected.txt:3
> > +FAIL Relationship between types and items assert_equals: expected ["text/plain"] but got ["text/plain"]
> 
> This error seems strange, we got what we expect so why are we failing?
As noted in the changelog, I think it's expecting === and it is only ==.  I tried tinkering with it for a while, but couldn't get it quite right in a simple patch.
Comment 8 Alex Christensen 2020-08-26 11:41:01 PDT
Created attachment 407314 [details]
Patch
Comment 9 Alex Christensen 2020-08-26 12:51:36 PDT
Created attachment 407325 [details]
Patch
Comment 10 EWS 2020-08-26 13:18:29 PDT
Committed r266187: <https://trac.webkit.org/changeset/266187>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407325 [details].
Comment 11 Radar WebKit Bug Importer 2020-08-26 13:40:48 PDT
<rdar://problem/67826601>