If you add an image field, and use the default "png, gif, jpg, jpeg" allowed extensions, this causes the file field instance code to output:

<input accept="png,gif,jpg,jpeg" type="file" id="edit-field-image-und-0-upload" name="files[field_image_und_0]" size="22" class="form-file">

When it should be:

<input accept="image/png,image/gif,image/jpg,image/jpeg" type="file" id="edit-field-image-und-0-upload" name="files[field_image_und_0]" size="22" class="form-file">

... as per: http://www.w3schools.com/tags/att_input_accept.asp

It appears that the file module is using this parameter to pass valid extensions to file.js. Unfortunately, some browsers like Chrome (at least on OS X), actually try to handle it. The result is that you can't select any file.

CommentFileSizeAuthor
#16 file_extension_validate.patch3.84 KBquicksketch
#5 939962.patch1.3 KBchx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rwohleb’s picture

rwohleb
he/him
Primary language English
CreditAttribution: rwohleb commented

I've taken another look at this and it's a simple fix in 'file.js'. Here are the modified functions:

Drupal.behaviors.fileValidateAutoAttach = {
  attach: function (context) {
    $('div.form-managed-file input.form-file[accept]', context).each(function () {
      $(this).data('accept', this.accept);
    }).removeAttr('accept').bind('change', Drupal.file.validateExtension);
  },
  detach: function (context) {
    $('div.form-managed-file input.form-file[accept]', context).unbind('change', Drupal.file.validateExtension);
  }
};

  validateExtension: function (event) {
    // Remove any previous errors.
    $('.file-upload-js-error').remove();

    // Add client side validation for the input[type=file] accept attribute.
    var accept = $(this).data('accept').replace(/,\s*/g, '|');
    if (accept.length > 1 && this.value.length > 0) {
      var acceptableMatch = new RegExp('\\.(' + accept + ')$', 'gi');
      if (!acceptableMatch.test(this.value)) {
        var error = Drupal.t("The selected file %filename cannot be uploaded. Only files with the following extensions are allowed: %extensions.", {
          '%filename': this.value,
          '%extensions': accept.replace(/\|/g, ', ')
        });
        $(this).parents('div.form-managed-file').prepend('<div class="messages error file-upload-js-error">' + error + '</div>');
        this.value = '';
        return false;
      }
    }
  },

Basically we just need to move the accept attribute into jquery data, then remove it so that the browser won't get confused. The only issue here would be browsers like Chrome falling back into broken functionality when JS is disabled.

rwohleb’s picture

rwohleb
he/him
Primary language English
CreditAttribution: rwohleb commented
Version: 7.0-beta1 » 7.0-beta2
Priority: Normal » Critical

I'm changing this to critical since this bug still exists in beta 2 and it really needs to be fixed before RC.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Version: 7.0-beta2 » 7.x-dev

Yep. This should be a comma-separated list of mimetypes.

http://www.w3.org/TR/html5/number-state.html#attr-input-accept

EvanDonovan’s picture

EvanDonovan CreditAttribution: EvanDonovan commented

@rwohleb: If you have a suggested fix, can you submit it as a .patch file so it can be tested by the testbot?

Ideally, the patch should be made using cvs diff so it is sure to apply.

chx’s picture

chx CreditAttribution: chx commented
Status: Active » Needs review
FileSize
939962.patch1.3 KB

Rolled a patch. Do not presume I know what i am doing.

idflood’s picture

idflood CreditAttribution: idflood commented

I'm not able to reproduce the issue on chrome 7.0.517.41 ( os x ). The output is like descibed in #0, with the "png,gif,jpg,jpeg" in the accept attribute but I still can upload images. I applied the patch provided in #5 and everything was still working ( tested on firefox 3.6.10, safari 5.0.2, chrome and webkit nightly )

chx’s picture

chx CreditAttribution: chx commented
Priority: Critical » Normal

Then, I think this is normal. If it can't be reproduced? Edit: it is not impossible that Chrome 7 fixed the problem which Chrome 6 had.

Shai’s picture

Shai CreditAttribution: Shai commented

The problem is in the Chrome 8 branch, not 7, which I think is worse for us and does indeed make the issue critical, but I'll leave it to others to change the status.

fuzzy76’s picture

fuzzy76 CreditAttribution: fuzzy76 commented

The problem is not in Chrome. The HTML5 spec states:

The accept attribute may be specified to provide user agents with a hint of what file types will be accepted.

If specified, the attribute must consist of a set of comma-separated tokens, each of which must be an ASCII case-insensitive match for one of the following:

The string audio/*
Indicates that sound files are accepted.
The string video/*
Indicates that video files are accepted.
The string image/*
Indicates that image files are accepted.
A valid MIME type with no parameters
Indicates that files of the specified type are accepted.

So listing file extensions there instead is clearly invalid HTML. No file will yield "jpg" as mimetype, so it's correct of the browser to refuse them. I don't think fixing invalid HTML with JS is a good solution.

This has also always been the case for HTML4: http://www.w3.org/TR/REC-html40/types.html#type-content-type

montesq’s picture

montesq CreditAttribution: montesq commented

One solution could be to modify the labels in order to ask entering type mime instead of extensions?
I.e.:
"Allowed file extensions" -> Allowed file mime types
"Separate extensions with a space or comma and do not include the leading dot.
File directory" -> separate mime types...

We could also put a link in the help label to a drupal.org documentation page where are listed mime-types and extensions (like http://www.webmaster-toolkit.com/mime-types.shtml)

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Active

mime types would be horrible, the simplest solution is to modify the code to not send accept at all.

EvanDonovan’s picture

EvanDonovan CreditAttribution: EvanDonovan commented

I agree with chx on this: we can't expect users to allow mime types. Could we match mime types to extensions during the rendering of the element? That way, we wouldn't need to rewrite using JS.

I don't think this should be considered critical if it is only in Chrome, which is a small percentage of the browser market. There are plenty of other issues that are more critical, which are only major priority currently.

Pedro Lozano’s picture

Pedro Lozano CreditAttribution: Pedro Lozano commented
Priority: Normal » Critical

As a user of Chrome I wouldn't believe D7 will be released with this bug. Currently I have to open Firefox every time I want to attach an image.

Is Chrome market share smaller than that of screen readers? There have been several critical issues for them.

I would prefer the "no accept attribute" solution before no solution at all.

Sorry if I have to bring this back to critical at least for a moment so you can just have a last minute thought about it.

This is also present in D6 filefield module #939102: Accept attribute contains extensions instead of mimetypes (preventing Chrome from selecting files)

EvanDonovan’s picture

EvanDonovan CreditAttribution: EvanDonovan commented
Priority: Critical » Major

Thanks for the link to D6 file field issue. I asked quicksketch over in that issue if he had insight over the best fix.

That said, I don't think this should be a release candidate blocker. You have the option to use another browser; screen reader users, often, do not.

As far as I know, there are 4 possible fixes for this issue:

a) Remove the accept attribute.
-- Advantages: easiest
-- Downsides: ?
b) Add a function that maps extensions to mime types, so that users can input extensions, but they would be output as mime types to the client.
-- Advantages: robust solution
-- Downsides: not sure what this would do for older browsers, not sure if people use FileField to allow file extensions that don't have a corresponding mime type (the array defining mime types could get quite long)
c) Do the swap of extensions for mime types client-side using JS.
-- Advantages: could do the fix only for the browsers that need it
-- Downsides: have to make sure the regex's were reliable, would probably be quite lengthy
d) Require users to enter mime types rather than extensions
-- Advantages: wouldn't require extra code (except in validation function?)
-- Disadvantages: horrible UX, not sure what this would do for older browsers

I'm not an expert on this issue, so I don't know which of these would be the best alternative. Hopefully, though, adding this list of options will help the relevant people make a decision on how to move forward.

quicksketch’s picture

quicksketch
he/him
CreditAttribution: quicksketch commented

Or a completely different solution that requires no effort on the end-user's part or any change in functionality:

e) Stop using the "accept" attribute entirely, and add the list of extensions as settings to the page with #attached['js'].

Users are completely incapable of entering MIME types, and even developers might be stumped as to what MIME types are appropriate for files like an XLSX or DOC file (oh of course, it's application/vnd.openxmlformats-officedocument.wordprocessingml.document!). All told the "accept" attribute isn't going to be much help to us as documented in the HTML5 spec unless you're working with a very small set of allowed types and if those types are already known (like image/jpg, image/png, and image/gif). Though even then you'll probably have troubles since who knows if we'll have problems with Windows (or IE more specifically) thinking image are image/pjpeg instead. Hard to say at this point.

In any case, let's just stop using the "accept" attribute. Contrib modules can attempt to solve this new problem with MIME types and the accept attribute.

quicksketch’s picture

quicksketch
he/him
CreditAttribution: quicksketch commented
FileSize
file_extension_validate.patch3.84 KB

Patch using method described in #15. Not as pretty as before, but JavaScript setting arrays never are friendly to read.

EvanDonovan’s picture

EvanDonovan CreditAttribution: EvanDonovan commented
Status: Active » Needs review

Ah, that was actually my option a), I think. :)

Setting to "needs review" for test bot & actual testers.

EvanDonovan’s picture

EvanDonovan CreditAttribution: EvanDonovan commented

@Pedro Lozano & the others who had marked this critical: have you tested quicksketch's patch yet? Testing the patch is the only way it will get into D7. I can't, since I don't have Chrome for OS X.

If you need help learning how to test a patch, go to http://drupal.org/patch/apply.

Pedro Lozano’s picture

Pedro Lozano CreditAttribution: Pedro Lozano commented

I tested the patch and it seemed to work, but I've seen some strange behavior with the file field that I think is not related to the patch.

Sometimes the validation error message doesn't disappear when you upload a correct file after an invalid one and there are other strange artifacts that are difficult to explain here.

You can see this things because they also happen in firefox.

EvanDonovan’s picture

EvanDonovan CreditAttribution: EvanDonovan commented

I think if one other person can confirm that this patch makes it possible to upload files in Chrome, and that it filters properly invalid vs. valid extensions, then it should be marked RTBC. Other problems should be discussed in a separate issue, if they aren't already.

Pedro Lozano’s picture

Pedro Lozano CreditAttribution: Pedro Lozano commented

Yes, I can confirm that it allows to select any file and the js validation of the extension works correctly. I'm using Chrome 9 on Mac.

bleen’s picture

bleen CreditAttribution: bleen commented

ignore this

EvanDonovan’s picture

EvanDonovan CreditAttribution: EvanDonovan commented

@bleen18 (or anyone else who has been following this issue):

If we could get one more confirmation that the patch fixes the issue, I think it could be RTBC. I can't test since I don't have a Mac which can run Chrome.

bleen’s picture

bleen CreditAttribution: bleen commented
Status: Needs review » Reviewed & tested by the community

I just tried the patch in #16 ...

I added a file field to "article" and only allowed png,jpg,gif files. On the create article form everything worked swimmingly. I was able to upload images without a problem, but the form yelled at me when I tried to upload a txt file.

...and it was good

FYI: I have a patch that NR at #939102: Accept attribute contains extensions instead of mimetypes (preventing Chrome from selecting files) ... that code definitely needs a closer look.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.