Bug 237219 – [macOS] Unable to upload ".pages" files to file inputs accepting ".pages" and ".jpeg" files

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Aditya Keerthi]

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

[Aditya Keerthi]

rdar://89482882

[Aditya Keerthi]

Created attachment 453240 [details]
Patch

[Tim Horton]

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`

[Aditya Keerthi]

(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.

[Aditya Keerthi]

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.

[Said Abou-Hallawa]

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.

[Aditya Keerthi]

Created attachment 453260 [details]
Patch for landing

[Aditya Keerthi]

(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.

[EWS]

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].

[Aditya Keerthi]

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