899519 - Use "description" instead of "mime type name" for filter name for accept attribute (input type=file)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Arnaud Bienner]

When working on bug 565274, I missed this attribute from nsIMIMEInfo.idl, which looks better in that case, as it's described  as "human readable".
So we should probably use this for the filter name which is shown to user, as mime type name is less meaningful.

Comment 1

[Arnaud Bienner]

Attachment: mime_type_description.patch

Comment 2

[Arnaud Bienner]

Comment on attachment 783097 [details] [diff] [review]
mime_type_description.patch

The idea here is to use description instead of mime type name.
E.g. this will shown "Adobe Acrobat Document" as the filter name instead of "application/pdf" currently; and "JPEG image" instead of "image/jpeg", which is really nicer IMO.

In this patch, I fallback on mime type name if description is empty, but I don't think this might ever happen for a valid mime type: we drop bad mime types and mime types with no extension. So I don't think we will ever have a valid mime type with extension set but no description set.

So maybe I should just always the description as the filter name.
This will make the code simpler.

Comment 3

[Arnaud Bienner]

Attachment: mime_type_description2.patch

A simpler version, as described above.
Maybe the comment is superfluous and we can just have "mimeInfo->GetDescription(filterName);" directly, as it might be self explanatory.

Comment 4

[Mounir Lamouri (:mounir)]

Comment on attachment 783097 [details] [diff] [review]
mime_type_description.patch

Review of attachment 783097 [details] [diff] [review]:
-----------------------------------------------------------------

That sounds good to me but I would like bz to review this because there might be MIME things I am not aware of.

Comment 5

[Mounir Lamouri (:mounir)]

Comment on attachment 783098 [details] [diff] [review]
mime_type_description2.patch

I would tend to prefer the other patch to be safe but I hope Boris to have some insights about the behaviour of .description.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 783097 [details] [diff] [review]
mime_type_description.patch

This looks fine.

There is no guarantee of description ever being nonempty, so you need some fallback no matter what.

Comment 7

[Arnaud Bienner]

Thanks for the review :)

Comment 8

[Arnaud Bienner]

Comment on attachment 783098 [details] [diff] [review]
mime_type_description2.patch

># HG changeset patch
># User Arnaud Bienner <[e-mail]>
># Date 1375183429 -7200
># Node ID 186deea69b5cc0827489bb6a3c829622838e7b4d
># Parent  a3c421763fbd8849bb86a5522578a3b7447dc636
>Bug 899519 - Use "description" instead of "mime type name" for filter name for accept attribute (input type=file)
>
>diff --git a/content/html/content/src/HTMLInputElement.cpp b/content/html/content/src/HTMLInputElement.cpp
>--- a/content/html/content/src/HTMLInputElement.cpp
>+++ b/content/html/content/src/HTMLInputElement.cpp
>@@ -6031,20 +6031,19 @@ HTMLInputElement::SetFilePickerFiltersFr
>       if (NS_FAILED(mimeService->GetFromTypeAndExtension(
>                       NS_ConvertUTF16toUTF8(token),
>                       EmptyCString(), // No extension
>                       getter_AddRefs(mimeInfo))) ||
>           !mimeInfo) {
>         continue;
>       }
> 
>-      // Get mime type name
>-      nsCString mimeTypeName;
>-      mimeInfo->GetType(mimeTypeName);
>-      CopyUTF8toUTF16(mimeTypeName, filterName);
>+      // Use the description as the filter name, as it's "human readable" (as
>+      // stated in the idl)
>+      mimeInfo->GetDescription(filterName);
> 
>       // Get extension list
>       nsCOMPtr<nsIUTF8StringEnumerator> extensions;
>       mimeInfo->GetFileExtensions(getter_AddRefs(extensions));
> 
>       bool hasMore;
>       while (NS_SUCCEEDED(extensions->HasMore(&hasMore)) && hasMore) {
>         nsCString extension;

Comment 9

[Arnaud Bienner]

Sorry, I forgot to update tje commit message when I provided the patch, and it looks like I can't modified it here.
You can use the bug's title (i.e. "Bug 899519 - Use "description" instead of "mime type name" for filter name for accept attribute (input type=file)") for the check-in.

Comment 10

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6367c753c7f9

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6367c753c7f9