Validate maximum picture dimensions [#131573]

Comment	File	Size	Author
#4	validate_max_dimensions.patch	1.55 KB	ksenzee

Comment #1

ricabrantes CreditAttribution: ricabrantes commented 11 March 2008 at 15:11

Version:

5.1

» 7.x-dev

Confirmed.. moving to new version..

Log in or register to post comments

Comment #2

LAsan CreditAttribution: LAsan commented 25 March 2008 at 12:13

Verified in 7.x-dev.

Log in or register to post comments

Comment #3

ksenzee

she/her

Seattle area

CreditAttribution: ksenzee commented 4 April 2008 at 14:56

Assigned:

Unassigned

» ksenzee

I'll take a stab at this...

Log in or register to post comments

Comment #4

ksenzee

she/her

Seattle area

CreditAttribution: ksenzee commented 4 April 2008 at 17:15

Status:

Active

» Needs review

File	Size
validate_max_dimensions.patch	1.55 KB

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.

Log in or register to post comments

Comment #5

lilou CreditAttribution: lilou commented 31 August 2008 at 04:47

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

Log in or register to post comments

Comment #6

lilou CreditAttribution: lilou commented 31 August 2008 at 04:50

Title:

Validate max dimensions

» Maximum dimensions for pictures more explicite

Log in or register to post comments

Comment #7

dmitrig01 CreditAttribution: dmitrig01 commented 31 August 2008 at 04:51

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().

Log in or register to post comments

Comment #8

lilou CreditAttribution: lilou commented 31 August 2008 at 04:54

Title:

Validate max dimensions

» Maximum dimensions for pictures more explicite

Cross-post for title ;-)

Log in or register to post comments

Comment #9

Dries CreditAttribution: Dries commented 31 August 2008 at 08:11

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.

Log in or register to post comments

Comment #10

drewish CreditAttribution: drewish commented 9 September 2008 at 02:29

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?

Log in or register to post comments

Comment #11

ksenzee

she/her

Seattle area

CreditAttribution: ksenzee commented 7 November 2008 at 22:56

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.