Bug 237219

Created attachment 453240 [details] Patch

Comment on attachment 453240 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453240&action=review > Source/WebCore/ChangeLog:27 > + While the aforementioned behavior can be considerd strange, it is not `considerd`

(In reply to Aditya Keerthi from comment #0) > 1. Install/open the latest Safari 15.4 beta > 2. Visit data:text/html,<input id="input" type="file" accept=".pages, > image/jpeg"> > 3. Attempt to upload a Pages file > 4. Observe that a zero byte JPEG is uploaded (In reply to Tim Horton from comment #3) > Comment on attachment 453240 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=453240&action=review > > > Source/WebCore/ChangeLog:27 > > + While the aforementioned behavior can be considerd strange, it is not > > `considerd` Thanks for the review! Will upload a patch for landing, after I hear back from Said.

Comment on attachment 453240 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453240&action=review > Source/WebCore/ChangeLog:9 > + In r227051, WebKit began transcoding images of an unsupported format This should be r264286 (https://trac.webkit.org/changeset/264286/webkit). 227051 is the canonical identifier (https://commits.webkit.org/227051@main). Will correct in the patch for landing.

Comment on attachment 453240 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453240&action=review > Source/WebCore/ChangeLog:19 > + The ".pages" extension does not have a well-defined MIME type in I think the bug should happen with all non image extensions and not necessarily ".pages" only, <input id="input" type="file" accept=".pdf, image/jpeg"> or <input id="input" type="file" accept=".txt, image/jpeg"> for example. > Source/WebCore/ChangeLog:49 > + Type Identifier for thte selected file using CGImageSourceCreateWithURL thte -> the > LayoutTests/fast/forms/file/entries-api/pages-jpeg-open-panel.html:8 > + <input id="input" type="file" accept=".pages, image/jpeg"> Should not we test pdf also? I think uploading PDF and images is a common scenario. > LayoutTests/fast/forms/file/entries-api/pages-jpeg-open-panel.html:25 > + shouldBeNonZero("file.size"); Should not we check that 'file.size' is equal to the size of the actual file? shouldBe("files.size", "89650"); // or whatever the size is.

Created attachment 453260 [details] Patch for landing

(In reply to Said Abou-Hallawa from comment #6) > Comment on attachment 453240 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=453240&action=review > > > Source/WebCore/ChangeLog:19 > > + The ".pages" extension does not have a well-defined MIME type in > > I think the bug should happen with all non image extensions and not > necessarily ".pages" only, <input id="input" type="file" accept=".pdf, > image/jpeg"> or <input id="input" type="file" accept=".txt, image/jpeg"> for > example. The bug only happens with extensions that don't have a well-defined MIME type (when MIME type for the extension is an empty string). ".pages" is one example, but ".abcd" and other custom extensions could run into the same issue. ".pdf" and ".txt" do not run into the issue, since they map to "application/pdf" and "text/plain" respectively. Since they don't map to empty strings, the lookup for accepted MIME types will succeed, and transcoding is already avoided. > > Source/WebCore/ChangeLog:49 > > + Type Identifier for thte selected file using CGImageSourceCreateWithURL > > thte -> the Fixed. > > LayoutTests/fast/forms/file/entries-api/pages-jpeg-open-panel.html:8 > > + <input id="input" type="file" accept=".pages, image/jpeg"> > > Should not we test pdf also? I think uploading PDF and images is a common > scenario. As mentioned earlier, the PDF case works fine before this change. But I've added a test for PDF anyway, since it's a common case, and is currently untested. > > LayoutTests/fast/forms/file/entries-api/pages-jpeg-open-panel.html:25 > > + shouldBeNonZero("file.size"); > > Should not we check that 'file.size' is equal to the size of the actual file? > > shouldBe("files.size", "89650"); // or whatever the size is. Done.

Committed r290606 (247879@main): <https://commits.webkit.org/247879@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453260 [details].

*** Bug 238668 has been marked as a duplicate of this bug. ***