Bug 237219 - [macOS] Unable to upload ".pages" files to file inputs accepting ".pages" and ".jpeg" files
Summary: [macOS] Unable to upload ".pages" files to file inputs accepting ".pages" and...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aditya Keerthi
URL:
Keywords: InRadar
: 238668 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-02-25 11:53 PST by Aditya Keerthi
Modified: 2023-06-23 08:39 PDT (History)
6 users (show)

See Also:


Attachments
Patch (66.90 KB, patch)
2022-02-25 11:57 PST, Aditya Keerthi
thorton: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (82.12 KB, patch)
2022-02-25 14:19 PST, Aditya Keerthi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aditya Keerthi 2022-02-25 11:53:39 PST
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
Comment 1 Aditya Keerthi 2022-02-25 11:54:00 PST
rdar://89482882
Comment 2 Aditya Keerthi 2022-02-25 11:57:05 PST
Created attachment 453240 [details]
Patch
Comment 3 Tim Horton 2022-02-25 12:44:02 PST
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`
Comment 4 Aditya Keerthi 2022-02-25 12:58:41 PST
(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 5 Aditya Keerthi 2022-02-25 13:00:01 PST
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 6 Said Abou-Hallawa 2022-02-25 13:37:03 PST
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.
Comment 7 Aditya Keerthi 2022-02-25 14:19:08 PST
Created attachment 453260 [details]
Patch for landing
Comment 8 Aditya Keerthi 2022-02-25 14:24:01 PST
(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.
Comment 9 EWS 2022-02-28 10:31:30 PST
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].
Comment 10 Aditya Keerthi 2023-06-23 08:39:05 PDT
*** Bug 238668 has been marked as a duplicate of this bug. ***