Download & Extend

Validate maximum picture dimensions

Project:Drupal core
Version:7.x-dev
Component:user.module
Category:bug report
Priority:normal
Assigned:ksenzee
Status:closed (won't fix)
Issue tags:Needs tests, user pictures

Issue Summary

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.

Comments

#1

Version:5.1» 7.x-dev

Confirmed.. moving to new version..

#2

Verified in 7.x-dev.

#3

Assigned to:Anonymous» ksenzee

I'll take a stab at this...

#4

Status:active» needs review

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.

AttachmentSizeStatusTest resultOperations
validate_max_dimensions.patch1.55 KBIgnored: Check issue status.NoneNone

#5

Title:Input checking missing and input format ambiguity» Validate max dimensions

You patch look good, but i think it need a .test for user_picture_dimensions

#6

Title:Validate max dimensions» Maximum dimensions for pictures more explicite

#7

Title:Maximum dimensions for pictures more explicite» Validate max dimensions
Status:needs review» reviewed & tested by the community

Works for me, and looks good. Much needed. Not sure if it needs a test because that's basically testing php's explode().

#8

Title:Validate max dimensions» Maximum dimensions for pictures more explicite

Cross-post for title ;-)

#9

Status:reviewed & tested by the community» needs work

1. It could use a test.

2.

+  if(empty($dimensions)) {
+    return;
+  }
It is not clear what this check is for. Wouldn't an empty dimension be a problem? The form description does not mention that it is a special case.

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

Title:Maximum dimensions for pictures more explicite» Validate maximum picture dimensions

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

#15

Writing a test today.

#16

Status:needs work» closed (won't fix)

Bug no longer relevant.

New UI has seperate fields for x and y.

New UI is at: http://www.d7c.dev/user/1/edit?destination=admin/people#overlay=admin/co...

Verify and close.