Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sulav’s picture

Status: Closed (fixed) » Needs review
mgifford’s picture

Version: 7.26 » 7.x-dev

Does this error apply to D8 as well? I would assume so but haven't checked. If so it would need to be fixed there too.

poker10’s picture

This is not relevant for D9, because in D9 the input type "number" is used and #min and #max properties are set-up (so this is being checked by separate validations). In D7 we are only using input type "text".

D9:

<input data-drupal-selector="edit-settings-max-resolution-x" type="number" id="edit-settings-max-resolution-x" name="settings[max_resolution][x]" value="" step="1" min="1" class="form-number">

D7:

<input type="text" id="edit-instance-settings-max-resolution-x" name="instance[settings][max_resolution][x]" value="" size="5" maxlength="5" class="form-text">

The patch looks good, however there is a redundant check for is_numeric():

@@ -176,6 +176,10 @@ function _image_field_resolution_validate($element, &$form_state) {
+  else if ((is_numeric($value)) && ($value < 0)) {

In this part the $value must be numeric, because in the first IF condition there is a return; in case the variable is not numeric.

I have updated the patch (removed the redundant check) and added a simple test for these dimensions, as it seems like we did not have any until now.

The last submitted patch, 3: 2248177-3_test-only.patch, failed testing. View results

yashingole’s picture

Assigned: Unassigned » yashingole
yashingole’s picture

Assigned: yashingole » Unassigned
Status: Needs review » Reviewed & tested by the community
FileSize
112.89 KB
79.01 KB

Verified and tested patch #3 on Drupal 7.x. Patch was applied successfully and looks good to me.
Testing steps:
1. Install 7.x
2. Navigate to admin/structure/types/manage/article/fields/field_image
3. Scroll Down to Image Resolution.
4. Set the values in negative.
5. Click on Save.
6. Changes are saved.
7. Apply the patch.
8. Navigate back to admin/structure/types/manage/article/fields/field_image
9. Set the resolution to negative.
10. Click on save.
11. Observe the error displayed.
Testing Result:
1. After applying the patch the error is displayed "Height and width values must be positive numbers."
Can be move to RTBC
Screenshots are attached for reference:

  • mcdruid committed 64b8a9a on 7.x
    Issue #2248177 by poker10, sulav: Image dimensions should accept only...
mcdruid’s picture

Status: Reviewed & tested by the community » Fixed

Great, thanks everyone!

Status: Fixed » Closed (fixed)

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