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

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla2.0b7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
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.
(Assignee)

Comment 1

8 years ago
Asking for being a blocker. We may want to ship Firefox 4 with that bug fixed.
blocking2.0: --- → ?
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.
Assignee: nobody → mounir.lamouri
blocking2.0: ? → final+
(Assignee)

Updated

8 years ago
Depends on: 598236
(Assignee)

Comment 3

8 years ago
Created attachment 477023 [details] [diff] [review]
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.
Attachment #477023 - Flags: review?(jonas)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
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.
Attachment #477023 - Flags: review?(jonas) → review+
(Assignee)

Comment 5

8 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/60b8e7db15a5
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.