"Conflicting" validators for user picture leads to confusing form error message

Hi Loziju,

I have tested the above patch in the Drupal 7 (Both dev and stable) version. The patch works as expected.

Now the irrelevant message (Only files with the following extensions are allowed: jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp) is replaced with (The specified file 160094-excavator-template-16x9.pptx could not be uploaded. Only JPEG, PNG and GIF images are allowed).

Hi Loziju,

Can you please commit the above changes and close this ticket?

It would be great if we can add a test for this. But I agree that these two error together are confusing.

This looks good to me. And it is not relevant for D9 - the user picture is a regular field here. +1 to RTBC.

------------

By adding the 'file_validate_extensions' => array() we are removing the allowed extensions check by this validator at all, see the file_save_upload():

// If 'file_validate_extensions' is set and the list is empty then the
// caller wants to allow any extension. In this case we have to remove the
// validator or else it will reject all extensions.
unset($validators['file_validate_extensions']);

But it seems like that this check is not required, because the same functionality is also provided by the 'file_validate_is_image' => array(). So it can't go beyond supported image extensions at any case.

This removal would also help this issue: #2539478: [D7] Allow image fields to use any extensions the current image toolkit supports (instead of hard-coding jpg, png and gif only). Currently the list of allowed extensions on this field was hardcoded and restricted by default values of file_validate_extensions() (see file_save_upload() and example below) and cannot be overrided (unlike the same check in the image_field_widget_form() which is using the extensions dynamically).

// No validator was provided, so add one using the default list.
// Build a default non-munged safe list for file_munge_filename().
$extensions = 'jpg jpeg gif png txt doc xls pdf ppt pps odt ods odp';
$validators['file_validate_extensions'] = array();
$validators['file_validate_extensions'][0] = $extensions;

So no easy way to allow for example .webp here. Developers needed to replace the whole validation function here: $form['#validate'][] = 'user_validate_picture';. After this change (and the related issue), all image extensions supported by the image toolkit will be allowed on that field.

-----------

I am reuploading the patch from #2 with the test added (patch itself is unchanged). It clearly demonstrates that there are two error messages in the specified case (before the patch).

I have also changed the generated file which is being uploaded in the testNoPicture() function. The reason is that actually for html type, there were two files returned by the drupalGetTestFiles() (first html-1.txt and second html-2.html). The test was incorrectly using the first .txt file instead of the correct .html one (.txt file does not trigger the additional validation message, so I have to correct it).

Just a note, this can be commited regardless of the related issues (it is not blocking them).

Verified and tested patch #8 on Drupal 7.x. Patch was applied successfully and looks good to me.
Testing steps:
1. Install 7.x
2. Navigate to People>edit the user.
3. Upload dcox type file in Picture.
4. Observe the error.
5. Apply the patch.
6. Repeat 2 and 3
7. Observe the error.
Testing Result:
1. After applying the patch the error is correctly displayed "The specified file Test Document.docx could not be uploaded. Only JPEG, PNG and GIF images are allowed.".
Can be move to RTBC
Screenshots are attached for reference:

I think this makes sense, and it's great to add the test... plus excellent if it makes it easier to support other valid image formats.

Worth adding a comment when we set the list of extensions to a blank array to explain why though, I think.

I'll do that on commit.

Thank you everyone!