Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
When uploading images bigger than allowed size (30kb) the success message is showing.
Proposed resolution
Do not display the The image was resized to fit within the maximum allowed dimensions of 85x85 pixels.
if the validation fails.
Remaining tasks
N/A
User interface changes
N/A
API changes
N/A
Comments
Comment #1
stoickthevast CreditAttribution: stoickthevast commentedComment #2
mgiffordI just noticed that too. It's both redundant & confusing. How can you get both a success message that an image was resized and a failure message that an image size was too large?
This was in 8.0.x as well.
Comment #3
dpiI can confirm this is still an issue. Rewording issue title.
File size for image fields is handled by
file_validate_size
. File rescale is handled byfile_validate_image_resolution
. Moving to file module.Comment #5
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedI couldn't see a great way to approach this as the `file_validate_image_resolution` function is effectively trying to do two things on it's own:
In the attached patch I modify the `file_validate` function to be able to handle either the default return values (ie, just an array of errors) or mixed return values of both messages and errors. It feels a bit dirty, perhaps someone else has a better suggestion.
Essentially the issue is `file_validate_image_resolution` is unaware of the results of other validations, yet we want the messages to only be shown if the other validations pass which is why I am passing the messages back to postpone sending of them until all validations are called.
Comment #7
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedUpdated test for `file_validate_image_resolution` to match new return format.
Comment #8
D-XPERT CreditAttribution: D-XPERT at SV Infotech commentedComment #9
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedComment #10
D-XPERT CreditAttribution: D-XPERT at SV Infotech commentedI have tested the patch in #7 with 8.4.x-dev, php 5.5 and mysql 5.5. The patch worked fine as per the proposed solution.
i.e. "Do not display the The image was resized to fit within the maximum allowed dimensions of 85x85 pixels. if the validation fails."
I also tried by uploading the image file of size less than 30KB with the success message of resized.
Note - This is the first time I reviewed a patch. Any feedback from community members are most welcome.
Comment #11
D-XPERT CreditAttribution: D-XPERT at SV Infotech commentedComment #12
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedThanks for the review!
Comment #13
tstoecklerI think this could be considered an API change, so I think we should introduce a BC-layer that checks if the return value of the validator contains a
'errors'
key and if not adjusts the format accordingly.Comment #14
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedIs this bit not what you mean?
It accepts both results (flat array or multi-dimensional array) so anyone adding a validator (including other image / file validators in core) continue to work fine without change.
Comment #15
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedComment #16
dpiThe format of the return value is different, therefore it is an API change and not permitted.
Comment #17
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedFair enough, I will try to rethink it. To me, the heart of the issue is that that validation function is trying to do two things at once.
Comment #18
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedSimply removing the resize save on validation (but still testing that it is re-sizeable) solves the issue.
Validation: Attempt to resize image but don't save.
Submit: Resize image and save.
Consequence of this change: Resize attempt happens twice, once in validation, once after submit.
The
file_validate_image_resolution()
describes it's own goal as "Verifies that image dimensions are within the specified maximum and minimum" and as it is a validator function, it should only do just that: validate. This removes the side effect.Comment #19
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedComment #21
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedWill wait for comments on the above before updating the tests to match the change.
Comment #22
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedAs the validation seems to need to actually generate the image (ie, do more than just validate), here is a different approach to ensure it validates only when there are no errors so no image generation occurs if there are any errors.
Comment #23
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedComment #25
scott_euser CreditAttribution: scott_euser at Fat Beehive commentedComment #35
AswathyAjish CreditAttribution: AswathyAjish commentedI am also facing the same issue.
My drupal version is 8.9.16.
Comment #36
artemboikoThe same on 9.2.7