Problem/Motivation

The Rotate class contains the following code.

    // PHP 5.5 GD bug: https://bugs.php.net/bug.php?id=65148: To prevent buggy
    // behavior on negative multiples of 90 degrees we convert any negative
    // angle to a positive one between 0 and 360 degrees.
    $arguments['degrees'] -= floor($arguments['degrees'] / 360) * 360;

Steps to reproduce

Proposed resolution

Remove all instances with the same comment

Remaining tasks

Review

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#24 Rectangle.patch5.37 KBaayushmankotia
#18 3104282-18.patch801 bytes_utsavsharma
#5 drupal-remove-php5-workaround-code-3104282-5.patch933 bytesavpaderno

Issue fork drupal-3104282

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kiamlaluno created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented

Duplicate of #2921123: Adjust Rectangle class to calculate rotated image dimensions according to libgd 2.2.2+. Would really be good to get reviews there.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

This issue is about the classes in the Drupal\system\Plugin\ImageToolkit\Operation\gd namespace, not the Drupal\image\Plugin\ImageEffect namespace. The classes in the first namespace don't use the Rectangle class.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new drupal-remove-php5-workaround-code-3104282-5.patch933 bytes

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Status: Needs review » Closed (outdated)
Related issues: +#2215369: Various bugs with PHP 5.5 imagerotate(), including when incorrect color indices are passed in

The comment and the related code was removed in #2215369: Various bugs with PHP 5.5 imagerotate(), including when incorrect color indices are passed in.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Closed (outdated) » Needs review

The comment is still present in the 10.1.x branch.

quietone’s picture

quietone commented

@apaderno, thanks. You are correct.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Removal doesn't seem to cause any issue with the 10.1 tests. LGTM

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Rotate.php
@@ -37,11 +37,6 @@ protected function arguments() {
   protected function validateArguments(array $arguments) {
-    // PHP 5.5 GD bug: https://bugs.php.net/bug.php?id=65148: To prevent buggy
-    // behavior on negative multiples of 90 degrees we convert any negative
-    // angle to a positive one between 0 and 360 degrees.
-    $arguments['degrees'] -= floor($arguments['degrees'] / 360) * 360;
-

We have exactly the same comment in \Drupal\Component\Utility\Rectangle::rotate() - should be removed there too.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

For those wondering, we have copious test coverage of rotating rectangles in \Drupal\Tests\Component\Utility\RectangleTest::providerPhp55RotateDimensions()

Tbh I think this provider should be renamed too - because the 5.5ness of the test coverage is irrelevant.

_utsavsharma’s picture

_utsavsharma commented
StatusFileSize
new 3104282-18.patch801 bytes

Made changes as per the comment #16.
Please review.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

The comment must be removed from two methods. I also take that alexpott said a test provider should be renamed too.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone opened merge request !6567

quietone’s picture

quietone commented
Title: Remove PHP 5 code » Remove PHP 5 code for GD
Issue summary: View changes
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Appears to have a test failure. Because quietone is a core committer can't rerun the tests but it was a Unit failure.

aayushmankotia’s picture

aayushmankotia commented
StatusFileSize
new Rectangle.patch5.37 KB

Hi,
I have created a patch as per comment #16. Please review.

quietone’s picture

quietone commented

@aayushmankotia, thank you for your interest in this issue.

Drupal is no longer using a patch workflow, all work is done in the merge request. See the top of this issue to find details about the merge request. The documentation on Drupal is in the process of being converted. I suggest you read Using GitLab to Contribute to Drupal. Thanks.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.