Patch attached provides a fix for the icon uploads. So far it hasn't broken anything.

Because:

  1. I had no idea whether it's necessary to exclude jpg as an allowed mime type or not, so I didn't
  2. file_validate_image_resolution will resize a too large icon if an image toolkit is available and reject the icon if one isn't or the icon is too small, and
  3. error messages are returned by file_validate_image_resolution and file_validate_is_image

the patch also:

- removes the extra file type and resolution validation in _outline_designer_settings_submit(), and

- alters the description of the upload form item (I don't much like the wording but it's the best I could come up with).

The patch is actually against HEAD not the 6.x-1 RC.

Cloudy

CommentFileSizeAuthor
#7 outline_designer_icon_uploads_fix_01.patch5.34 KBCloudCuckoo
outline_designer_icon_uploads_fix.patch3.14 KBCloudCuckoo

Comments

btopro’s picture

btopro commented

I'll try it out tomorrow and see, thanks in advance for figuring this out. I tried fixing it for awhile and got ticked off and gave up as it wasn't a high priority issue

btopro’s picture

btopro commented
Status: Needs review » Needs work

I applied the code and got the following message:

"The selected file section - reference based associations.jpg could not be uploaded. The image is too small; the minimum dimensions are 16x16 pixels."

The file I uploaded was very large so I'm not sure what the deal is, were you able to get this patch to work with your site?

CloudCuckoo’s picture

CloudCuckoo commented

It's because I included minimum dimensions in file_validate_image_resolution and the resized image was smaller then 16x16, sadly the core error message doesn't make it clear that's what happened.

Is it necessary for the images to be exactly 16x16? Because dimensions are optional the minimum dimensions can be removed.

btopro’s picture

btopro commented

No the image I uploaded was like 300x200 or something and it gave that message so I'm not sure why. the 16x16 is important because when you view the outline designer it needs to make the rowsuniform in height / width so that everything lines up correctly. Scaling to 16x16 is fine (and I like that solution) but it is necessary for usability's sake.

CloudCuckoo’s picture

CloudCuckoo commented

The error message appears because a 300x200 image file_validate_image_resolution() resizes it to 16x11 to pass the maximum resolution check which means it will then fail the minimum resolution check of 16x16.

I'm guessing because file_validate_image_resolution() calls image_scale() which maintains the aspect ratio if the original image isn't square (height=width) its gonna fail.

(see http://api.drupal.org/api/function/file_validate_image_resolution/6 and http://api.drupal.org/api/function/image_scale/6)

Foolishly I had only tested this patch with square images. I have an idea that may redeem me, another patch will be along shortly.

btopro’s picture

btopro commented

Sounds good to me, I'll be sure to roll it in asap, thanks for hunting this down!

CloudCuckoo’s picture

CloudCuckoo commented
Assigned: Unassigned » CloudCuckoo
Status: Needs work » Needs review
StatusFileSize
new outline_designer_icon_uploads_fix_01.patch5.34 KB

Here we go again. I was trying to reduce not introduce code. I failed.

Patch introduces a new validation callback for file_save_upload() : _outline_designer_validate_image_resolution().

When a toolkit is available it will scale and crop the uploaded image to fit the necessary dimensions. Works in both directions i.e. scales too small images up and scales too big images down. If no toolkit is available it should fail - I haven't tested that.

You can see this patch at work at http://sandbox.imaginaryplaygrounds.com. You need to log in as administrator, login info is on the site.

btopro’s picture

btopro commented
Status: Needs review » Fixed

Great work! I tried out your demo then patched my own version and it worked. i've gotta move the files to my computer I use for cvs and it'll be up in RC2

Status: Fixed » Closed (fixed)

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