Problem/Motivation

I see that the module follows very old PHP standard practices, for example it doesn't follow the function return type hinting, among other features that need to be added

More information about Drupal's coding standards

Steps to reproduce

Look into the module's source code, it is using outdated PHP practices.

Proposed resolution

Upgrade the module follow recent standard practices for writing more robust PHP code.

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#12 Screenshot from 2026-05-09 11-41-31.png71.97 KBdanrod

Issue fork media_thumbnails_svg-3581197

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

danrod created an issue. See original summary.

danrod’s picture

danrod
Location Ottawa
commented
Issue tags: +Novice
danrod’s picture

danrod
Location Ottawa
commented

I'll have to port this to the 3.x branch as well.

sapnil_biswas made their first commit to this issue’s fork.

sapnil_biswas’s picture

sapnil_biswas commented
Assigned: Unassigned » sapnil_biswas

Starting to work on this

sapnil_biswas opened merge request !7

sapnil_biswas’s picture

sapnil_biswas commented
Assigned: sapnil_biswas » Unassigned
Status: Active » Needs review

marking this as needs review please let me know if there is requirement for any further changes

danrod’s picture

danrod
Location Ottawa
commented
Title: Edit Better PHP Standards » Better PHP Standards
vinodhini.e’s picture

vinodhini.e commented

Hi, Tested this module on Drupal 11.2.5 and ran coding standards (PHPCS and PHPStan).

After applying MR !7, I re-ran the checks, but issues are still being reported.

PHPCS warnings:

File: src/Hook/MediaThumbnailsSvgHooks.php
Line 21: t() call used in class; should use StringTranslationTrait and $this->t()
Line 22: t() call used in class; should use StringTranslationTrait and $this->t()

PHPStan errors:

File: src/Plugin/MediaThumbnail/MediaThumbnailSVG.php
Line 24: Extends unknown class MediaThumbnailBase
Line 50: Unknown class FileRepositoryInterface
Line 55: Invalid return type MediaThumbnailBase
Line 56: Calls parent::create() but no valid parent class
Line 73: Undefined property $fileSystem
Line 75: Undefined property $logger and undefined method t()
Line 84: Function _media_thumbnails_svg_has_graphics_magick not found
Line 87: Function _media_thumbnails_svg_has_image_magick not found
Line 100: Class FileInterface not found
Line 170: Unknown class SVG\SVG
Lines 172, 188, 195, 216: Undefined property $logger and undefined method t()

Please refer to the detailed error output above. The MR does not fully resolve the coding standard issues yet. Thanks.

sapnil_biswas’s picture

sapnil_biswas commented

Thanks @vinodhini.e for testing this out. I have resolved all the remaining warnings checked locally and the pipeline is green now

vinodhini.e’s picture

vinodhini.e commented

Hi, @sapnil_biswas,

Applied the patch and re-ran PHPCS. No errors are reported, and it is working as expected. Thanks.

danrod’s picture

danrod
Location Ottawa
commented
StatusFileSize
new Screenshot from 2026-05-09 11-41-31.png71.97 KB

I tested this on my D11.3 instance and works as expected (screen attached) moving this to RTBC.

danrod’s picture

danrod
Location Ottawa
commented
Status: Needs review » Reviewed & tested by the community

danrod’s picture

danrod
Location Ottawa
commented

I had to revert the file media_thumbnails_svg.info.yml to support older Drupal versions in the 2.x branch.

diff --git a/media_thumbnails_svg.info.yml b/media_thumbnails_svg.info.yml
index be938c9..dea103c 100644
--- a/media_thumbnails_svg.info.yml
+++ b/media_thumbnails_svg.info.yml
@@ -1,7 +1,7 @@
 name: Media Thumbnails SVG
 type: module
 description: Generate custom thumbnails for svg media entities
-core_version_requirement: ^11.2 || ^12
+core_version_requirement: ^9.3 || ^10 || ^11
 package: Media
 dependencies:
   - media_thumbnails:media_thumbnails

I also changed the file MediaThumbnailSVG.php to use the constant FileSystemInterface::EXISTS_REPLACE to support older Drupal versions

Tested on a D10 instance, worked without issues.

I'll port this to the 3.x branch.

danrod’s picture

danrod
Location Ottawa
commented

Ported to 3.x and tested on a D11.2 instance, with no issues, I'll mark this as Fixed. Thanks a lot everyone !

danrod’s picture

danrod
Location Ottawa
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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