Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Follow-up to #2420789: Make RotateImageEffect's random rotation deterministic, so ::transformDimensions() can return the actual dimensions where this bug was discovered during review.
Problem/Motivation
The rotate image effect has a parameter degrees/angle which only accepts integers. This should be changed to float because of the following reasons:
- By their very nature an angle is better described by a float.
- D7 does accept floats. Although this can/should also be considered a bug because the validation on the degrees form field in D7 is function image_effect_integer_validate() which appear to accept floats! This means that there will be D7 sites that use styles that have a rotate effect with a non integer number for degrees (I have created some to start with). So we better accept that and change our validation and scheme settings in D8 and D7.
Proposed resolution
D8;
- Allow floats for the angle parameter of the rotate image effect.
D7:
- Allow floats for the angle parameter of the rotate image effect.
- Change the validation on the form element to element_validate_number().
- Correct function image_effect_integer_validate() to only accept integers.
Remaining tasks
- write patch
- review patch
- accept patch
- backport to D7, where we should choose between leaving the current image_effect_integer_validate() function as is (and hope that the calling code does some casting) or make it stricter.
User interface changes
None in D8, Better error messages in D7 (I now get "Rotation angle must be a positive integer." when I enter -4,5, a typical often made mistake for a.o dutch users, as dutch uses the comma as decimal separator)
API changes
None, unless loosening scheme and parameter types are considered an API change.
Data model changes
None
Comments
Comment #2
fietserwinComment #13
catchThis seems like a Drupal 7 bug rather than a Drupal 8 one, and if anything Drupal 7 should fix the bad data or warn people about it. Needs more explanation of why it's a bug, corrupted configuration in Drupal 7 isn't a reason to do the same in 9.x.
Comment #15
quietone CreditAttribution: quietone at PreviousNext commentedSince we need more information to move forward with this issue, and none has been supplied I am closing this issue.
If you are experiencing this problem on a supported version of Drupal reopen the issue, by setting the status to 'Active', and provide complete steps to reproduce the issue (starting from "Install Drupal core").