Bug 215837 - Implement DataTransfer constructor and multipart form filename encoding as other browsers do
Summary: Implement DataTransfer constructor and multipart form filename encoding as ot...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-25 20:59 PDT by Alex Christensen
Modified: 2020-08-26 13:40 PDT (History)
8 users (show)

See Also:


Attachments
Patch (13.20 KB, patch)
2020-08-25 21:01 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (13.15 KB, patch)
2020-08-25 21:07 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (20.04 KB, patch)
2020-08-26 09:51 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (21.79 KB, patch)
2020-08-26 11:41 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (21.75 KB, patch)
2020-08-26 12:51 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>