590386 - <input type='file'>'s filepicker shouldn't filter files if more than one token is set in @accept

Status: RESOLVED FIXED

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

Description

[Mounir Lamouri (:mounir)]

For exmample:
<input type='file' accept="video/*, audio/*">'s filepicker shouldn't filter files. Currently, we can filter audio OR video but not both of them at the same time (depending on the platform, the user can change the filter in the UI).

To prevent any confusion, we shouldn't filter in that situation.

Comment 1

[Mounir Lamouri (:mounir)]

Asking for being a blocker. We may want to ship Firefox 4 with that bug fixed.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Marking blocking as this is a bug in a new feature that could prevent websites from using it.

Blocking final as this should be a trivial and non-risky fix.

Comment 3

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

So, with this patch, if there is more than one known token (ie. "audio/*", "image/*" and "video/*"), no filter will be used.

One edge cases is managed: if the same token is repeated, it will be used (we never know...). Example: accept="audio/*, audio/*", it will filter for audio files.

Filtering with extensions is completely ignored. That means accept="audio/*, *.avi" will filter audio files.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 477023 [details] [diff] [review]
Patch v1

>     if (token.EqualsLiteral("image/*")) {
>-      filters |= nsIFilePicker::filterImages;
>+      if (filter && filter != nsIFilePicker::filterImages) {
>+        return 0;
>+      }
>+      filter = nsIFilePicker::filterImages;
>     } else if (token.EqualsLiteral("audio/*")) {
>-      filters |= nsIFilePicker::filterAudio;
>+      if (filter && filter != nsIFilePicker::filterAudio) {
>+        return 0;
>+      }
>+      filter = nsIFilePicker::filterAudio;
>     } else if (token.EqualsLiteral("video/*")) {
>-      filters |= nsIFilePicker::filterVideo;
>+      if (filter && filter != nsIFilePicker::filterVideo) {
>+        return 0;
>+      }
>+      filter = nsIFilePicker::filterVideo;
>     }

Nit: It'd be less code to do something like:

PRInt32 tokenFilter = 0;
if (token.EqualsLiteral("image/*")) {
  tokenFilter = ...;
} else ...
}

if (filter && tokenFilter && filter != tokenFilter) {
  return 0;
}
filter = tokenFilter;

r=me either way.

Comment 5

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/60b8e7db15a5