File validate image resolution inaccurate in some situations

Definitely needed.

Patch applies but with offset.

Comments changes/additions that are not completely necessary but are not harmful at all. Cleanup that is not essential to this patch though. I'm all for leaving them there, just wanted to make sure it was pointed out.

+ * (optional) A string in the form WIDTHxHEIGHT. This will check that the

+ * (optional) A string in the form WIDTHxHEIGHT e.g. '640x480' or '85x85'. If

+ // Clear the cached filesize and refresh the image information.

These two lines will throw an exception if min or max dimension is 0, which is what it is set to by default.

+  list($max_width, $max_height) = explode('x', $maximum_dimensions);
+  list($min_width, $min_height) = explode('x', $minimum_dimensions);

Fixing that should fix at least some of the exceptions encountered by the testing bot.

Should this include a test for this new validation case? Make sure that an image that can not be resized to the appropriate dimensions is rejected.

Is that "(optional)" change based off some new standard for core? I've not seen it before and it doesn't match the rest of the file. If it's not a core initiative I'd leave it as it was.

I find these changes pretty confusing and hard to follow, not to mention untested. I wonder if it would be clearer to—as was discussed over on #353580: file_validate_image_resolution doesn't allow not resizing if a toolkit is present.—break this into two functions one for validating and one for resizing.

Related: #27234: Maximum picture size help text in profile no longer needed

Here is an updated version of the patch for latest drupal 7.

It also has a couple of additions to stop errors when $minimum_dimensions or $maximum_dimensions is zero, which addresses #2.

I don't have time right now to do tests sorry.

It fixes my issue, which was that I couldn't restrict a field to specific dimensions by filling out min & max res fields with the same value. Before it would resize the image down, not complying with the min value but not failing validation.

Here is a drupal 8 version of the patch in #6.

It is a direct port and I have not actually tested it, I only tested the D7 one.

Oops, here is a drupal 7 one to fix the exceptions.

The fails are due to the tests needing modification, but I'm going to bed instead of fixing those right now.

And the drupal 8 one.

Turns out I was wrong about the fails :)

Partial duplicate of http://drupal.org/node/1577554

Fixing tags per http://drupal.org/node/1517250

Perhaps I don't understand the D7 workflow, but why hasn't this made it into D7? Can we make that happen? Should I open a new issue? I see that in #7 it was moved to D8, unclear to me why.

The process is that if a bug also exists in drupal 8 it should be fixed there first, then once that happens the version will be set back to drupal 7 and it can be fixed there too.

There are patches for both drupal 8 & 7 though so hopefully this can be addressed.
To do that we need some patch reviews so if you can try out one or both of the patches and report back with the results that would help.

- Changing the owner seeing as quicksketch's last reply was in '09.

The patches in #9 (drupal 7) and #10 (drupal 8) are code-wise identical. The only diffs are the actual line numbers. I successfully applied the drupal 8 patch in #10 to my drupal 7 instance, and things appear to be working correctly for me.

This patch has been in place on my site since March 2013. Switching to "reveiwed & tested by community".

drupal-file_validate_img_res-516790-10.patch no longer applies.

error: patch failed: core/includes/file.inc:1834
error: core/includes/file.inc: patch does not apply

Re-rolled

new patch

Sorry, new version of patch which can be applied to latest 8.x core.

Version for Drupal 7.

New patch with tests.

+++ b/core/modules/file/src/Tests/ValidatorTest.php
@@ -73,6 +73,10 @@ function testFileValidateImageResolution() {
+    $errors = file_validate_image_resolution($this->image, '88x100', '88x100');

So the only thing I ask in return for RTBCing this patch, is that we file a follow-up to not use the Druplicon image in this test, and instead generate a test image on demand using Random::image() for a something more consistent for testing. Could you please file a follow-up issue for that?

The test does not fail with only the changes to tests applied - therefore we don't proper test coverage.

Fix media tag.

This patch is a the same as #24 but for 8.1.x

I tested this on Drupal 10.0.x, standard install. I adjusted the min and max image sizes for the image in the article content type and then attempted to load an image that when scaled down would be too small. I tested with different images and sizes and received error messages when the image was too small.

Therefore, closing as cannot reproduce. If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue (starting from "Install Drupal core").

Thanks!