Validate maximum picture dimensions
charles_elwood - March 27, 2007 - 20:33
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | user.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | ksenzee |
| Status: | needs work |
| Issue tags: | Needs tests, user pictures |
Description
The /admin/user/settings page has an option for 'Maximum Picture Dimensions' and helpfully states that the measurement is in pixels.
Say a user requires a maximum image size of 100 x 100 pixels, what gets put in the box?
100
100x100
100*100
100,100
Not knowing what to use, the first thing a user might try is to guess at some reasonable input and have the system provide helpful feedback if the input is not vaild.
'foo' Is not a sensible dimension, yet when used in that field the module accepts it as perfectly valid.
#1
Confirmed.. moving to new version..
#2
Verified in 7.x-dev.
#3
I'll take a stab at this...
#4
This patch adds a validator and some explanatory text for the maximum picture dimensions field. If you enter 'foo' or '80', it throws an error. '100x125' is accepted. A better approach might be to have two separate textfields, one for width and one for height, but this patch at least solves the immediate problem.
#5
You patch look good, but i think it need a .test for user_picture_dimensions
#6
#7
Works for me, and looks good. Much needed. Not sure if it needs a test because that's basically testing php's explode().
#8
Cross-post for title ;-)
#9
1. It could use a test.
2.
+ if(empty($dimensions)) {+ return;
+ }
3. There are some minor code style glitches.
#10
i kind of think it'd be worth the trouble to add a dimensions form element. we could use it several places in core and i can think of a few contrib modules that could use it. would that be worth pursuing?
#11
I agree it could use a test. I'll come up with one and address the other issues in #9. However, I also like the idea of adding a dimensions form element to core, and since I just figured out hook_elements 5 minutes ago (ahem) I could maybe whip one up. It would definitely be neater than the current patch. If I don't hear anything to the contrary I'll go ahead with that approach.
#12
ksenzee, please do and post a link so i can follow up on that issue. i've been thinking more and more that it'd be handy to have.
#13
Adding tags
#14
This looks like it's related to this issue:
#590616: 0x0 Image Filesize Shouldn't Come With 0x0 Warnings