Comments

MrPaulDriver created an issue. See original summary.

nick dewitte’s picture

nick dewitte’s picture

Status: Active » Needs review
mrpauldriver’s picture

Thank you for working on this.

After submission I am presented with two error messages and the image is not uploaded. At the end on the messages is the statement "The file filename.jpg with resolution 3586x1978 could not be uploaded. The resolution should be between x and 1920x1080"

Error One
Undefined offset: 1 in Drupal\media_bulk_upload\Form\MediaBulkUploadForm->validateImageResolution() (line 469 of modules/contrib/media_bulk_upload/src/Form/MediaBulkUploadForm.php).

Error Two
Undefined offset: 1 in Drupal\media_bulk_upload\Form\MediaBulkUploadForm->validateImageResolution() (line 487 of modules/contrib/media_bulk_upload/src/Form/MediaBulkUploadForm.php).

The full error message output can be found on pastebin.

nick dewitte’s picture

nick dewitte’s picture

Minor improvements:

  • Used short array syntax.
  • Added additional empty checks.
fonant’s picture

Updated patch that applies to latest code.

Would be nice to apply the checks using the client side JavaScript too.

fonant’s picture

And another minor fix to change the size error messages to say "than" instead of "then".

fonant’s picture

FanisTsiros’s picture

As per #4 this does not work.
Logic here, is that images have to scale down to maximum allowed image size at image field settings.
Just comes with an error saying that "The file nz6_3733.jpg with resolution 6048x4024 could not be uploaded. The file should be smaller than 1600x1600"

nick dewitte’s picture

#4 details a mistake in the error message.

This patch is intended to prevent users from uploading images larger than the maximum allowed size.

Before this patch, images where uploaded directly, even if their size was larger then the maximum size.

FanisTsiros’s picture

@Nick Dewitte, oh yes, sorry about that. I just reviewed the patch code and i relize this was not what i expected.
What i was expected was an image dimensions resize message like this:
The image was resized to fit within the maximum allowed dimensions of 1600x1600 pixels. The new dimensions of the resized image are 1600x1065 pixel
Seems that this is a core issue https://www.drupal.org/project/drupal/issues/3008292.
I am not sure if this patch here could be merged, so we have the functionality i wish.
Thank you for this module and your work!

vinaySreedhara’s picture

Patch looks good, but not adding the validation as it does check for type 'image' by using array key exists in the MediaType object so update the code to check the plugin id of the mediaType source.

rhristov’s picture

I am applying a new patch that is using the core image resolution validation and is resizing the image if needed.

ashu1629’s picture

Version: 8.x-1.x-dev » 3.0.0
StatusFileSize
new2.13 KB

I am applying new patch to work with 3.0.0 version. Please review. It works fine for max image resolution, but does not show error message for minimum image resolution.

kristen pol’s picture

Assigned: Unassigned » kristen pol

This is a nice UX issue. Assigning to myself to try to get this into the next release.

kristen pol’s picture

Fyi, for all the people who've added updated patches above, in the future, please include interdiffs so we know easily what has changed:

https://www.drupal.org/node/1488712

kristen pol’s picture

StatusFileSize
new1.22 KB
new3.36 KB
new1.56 KB

Here are crude raw diffs between the 4 patches.

kristen pol’s picture

Assigned: kristen pol » Unassigned
Status: Needs review » Needs work

Quick review of the last patch in #15. I haven't tested.

  1. +++ b/src/Form/MediaBulkUploadForm.php
    @@ -482,6 +483,12 @@ protected function processFile(MediaBulkConfigInterface $mediaBulkConfig, FileIn
    +       throw new \Exception('File entity could not be created.');
    

    Nitpick: missing space before throw

  2. +++ b/src/Form/MediaBulkUploadForm.php
    @@ -536,6 +543,31 @@ protected function validateFileSize(MediaTypeInterface $mediaType, FileInterface
    +   *   Media Type.
    

    Nitpick: The media type.

  3. +++ b/src/Form/MediaBulkUploadForm.php
    @@ -536,6 +543,31 @@ protected function validateFileSize(MediaTypeInterface $mediaType, FileInterface
    +   * @param array $file
    +   *   File.
    

    Fix param type: array => File

    Nitpick: The file.

kristen pol’s picture

Assigned: Unassigned » kristen pol

I'm going to fix this one and test it.

kristen pol’s picture

Assigned: kristen pol » Unassigned
Status: Needs work » Fixed

I tested and did some tweaks to the code. I found that it is fine for dealing with the max resolution but not the min which was noted above in #15. But this may be due to:

#2922667: Image fields minimum dimension restrictions don't apply equally in D7 and D8

so I'm going to let this slide for now. It does resize the image when it's too big which is nice.

Thanks, everyone!

Status: Fixed » Closed (fixed)

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