[Webkit-unassigned] [Bug 185416] Empty <input type=file> is represented incorrectly in FormData

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=185416

--- Comment #9 from Andreu Botella <andreu at andreubotella.com> ---
Comment on attachment 444343
  --> https://bugs.webkit.org/attachment.cgi?id=444343
Patch

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

>> Source/WebCore/html/FileInputType.cpp:182
>> +    // application/octet-stream content type. Null would be more logical, but
> 
> I don’t think this comment change is an improvement; mentioning exactly what the code does is a non-goal for comments. The comment needs to cover "why", but no reason for it to say exactly "what".

I read "empty" as meaning "with the default settings", which would include an empty content-type. But you're right, the code makes that clear anyway. I'll revert this change.

>> Source/WebCore/html/FileInputType.cpp:189
>> +            emptyString());
> 
> This vertical formatting is not normally WebKit coding style, although it is technically allowed. I would prefer not to switch to it.
> 
> Writing "application/octet-stream" here isn’t as good as the WebCore::defaultMIMEType() function, which is more efficient than creating a string from a C literal each time, and it's also good to not have to worry about typos. We should use that function here.
> 
> As a follow-up, someone should also use that function in at least: WebCore::FileReaderLoader::convertToDataURL, WebCore::FormData::appendMultiPartFileValue, WebCore::XMLHttpRequest::overrideMimeType, and WebKit::LegacyDownloadClient::legacyDidStart. And we should also consider making a function for comparing the string for use in suggestedFilenameWithMIMEType and WebKit::NetworkResourceLoader::didReceiveResponse.

I think this code was reformatted by my IDE, using clang-format I believe, and since webkit-patch did not complain, I assumed that was fine. My bad for not actually checking that it followed the coding style. I suppose the right way would be having all the parameters in one line?

I did not know that WebCore::defaultMIMEType() was a thing.

Should I open a new bug for these changes?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211117/318cc1b5/attachment.htm>