Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates image-related functionality out of the Yii2 adapter layer into the main CraftCms\Cms\Image namespace, adds an Images service + facade, and updates core/legacy callers and tests to use the new APIs.
Changes:
- Introduces new image domain classes (
Images,ImageHelper,Svg, enums) and anImagesfacade. - Refactors legacy Yii2 image service/helpers to act as deprecated proxies/aliases to the new implementations.
- Moves/rewrites image tests and fixtures into the main test suite (Feature/Unit), removing Yii2-adapter unit tests.
Reviewed changes
Copilot reviewed 41 out of 59 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| yii2-adapter/tests/unit/services/images/ImagesTest.php | Removed Yii2-adapter unit tests for the legacy images service. |
| yii2-adapter/tests/unit/services/images/.gitignore | Removed sandbox ignore (legacy test sandbox no longer used). |
| yii2-adapter/tests/unit/services/ImageTransformsTest.php | Removed Yii2-adapter unit tests for legacy transform helper behavior. |
| yii2-adapter/tests/unit/helpers/ImageHelperTest.php | Removed Yii2-adapter unit tests for legacy image helper functions. |
| yii2-adapter/tests/_data/assets/files/png-that-is-svg.png | Removed legacy fixture file. |
| yii2-adapter/tests/_data/assets/files/no-svg.svg | Removed legacy fixture file. |
| yii2-adapter/legacy/web/assets/cp/CpAsset.php | Uses new Images facade for isImagick Craft CP bootstrap data. |
| yii2-adapter/legacy/services/Images.php | Converts legacy service into a deprecated proxy to the new CraftCms\Cms\Image\Images service. |
| yii2-adapter/legacy/image/SvgAllowedAttributes.php | Replaces legacy implementation with alias/deprecation bridge to new SvgAllowedAttributes. |
| yii2-adapter/legacy/image/Svg.php | Replaces legacy SVG class with alias/deprecation bridge to new Svg. |
| yii2-adapter/legacy/helpers/ImageTransforms.php | Deprecated helper now delegates to CraftCms\Cms\Image\ImageTransformHelper (includes phpdoc tweaks). |
| yii2-adapter/legacy/helpers/Image.php | Deprecated helper now delegates to CraftCms\Cms\Image\ImageHelper and maps EXIF constants to enum values. |
| yii2-adapter/legacy/helpers/Assets.php | Updates image editor source generation to use the new Images facade. |
| yii2-adapter/legacy/base/Image.php | Replaces legacy base image class with alias/deprecation bridge to new CraftCms\Cms\Image\Image. |
| yii2-adapter/legacy/base/ApplicationTrait.php | Marks getImages() as deprecated in favor of the new images service. |
| tests/_data/assets/files/random.tiff | Adds TIFF fixture for ImageHelper/image-size/exif capability tests. |
| tests/_data/assets/files/random.tif | Adds .tif fixture (used for negative EXIF capability cases). |
| tests/_data/assets/files/no-dimension-svg.svg | Adds SVG fixture without width/height attributes (viewBox-only). |
| tests/_data/assets/files/invalid-ihdr.png | Adds malformed PNG fixture to exercise PNG parsing error paths. |
| tests/_data/assets/files/ign.jpg | Adds JPEG fixture used by PNG metadata tests (negative case). |
| tests/_data/assets/files/gng.svg | Adds SVG fixture used by SVG size parsing tests. |
| tests/_data/assets/files/dirty-svg.svg | Adds SVG fixture with unsafe tags to verify sanitization behavior. |
| tests/Unit/Image/ImageTransformHelperTest.php | Adds provider-based coverage to ensure extendTransform() matches legacy cases. |
| tests/Feature/Image/SvgTest.php | Adds feature tests for SVG load/resize/crop/save behavior. |
| tests/Feature/Image/RasterTest.php | Adds feature tests for raster load/resize/crop and SVG rasterization (Imagick-gated). |
| tests/Feature/Image/ImagesTest.php | Adds feature tests for the new Images service (sanitization, EXIF, GIF config, memory checks). |
| tests/Feature/Image/ImageHelperTest.php | Adds feature tests for ImageHelper (dimensions, web-safe, PNG metadata, streams, EXIF cleanup). |
| src/Utility/Utilities/SystemReport.php | Uses app(Images::class) instead of Craft::$app->getImages() when reporting image driver. |
| src/User/Users.php | Switches user photo validation from legacy craft\helpers\Image to new CraftCms\Cms\Image\ImageHelper. |
| src/Twig/Variables/Image.php | Updates Twig variable dependency to new CraftCms\Cms\Image\ImageHelper. |
| src/Support/Html.php | Switches SVG sanitizer allowed-attributes reference to the new CraftCms\Cms\Image\SvgAllowedAttributes. |
| src/Support/Facades/Images.php | Introduces Laravel facade for CraftCms\Cms\Image\Images. |
| src/Image/SvgAllowedAttributes.php | Adds allowed-attributes implementation for SVG sanitization. |
| src/Image/Svg.php | Adds new core SVG image implementation. |
| src/Image/Images.php | Adds new singleton images service (driver detection, load/clean, EXIF rotation/stripping, format support). |
| src/Image/ImageTransformer.php | Updates to use new ImageHelper/Images service for image editing and transform eligibility checks. |
| src/Image/ImageTransformHelper.php | Updates to use new ImageHelper/Images service and updates type docs for returned image object. |
| src/Image/ImageHelper.php | Adds new core image helper (dimensions, web-safe, PNG header parsing, stream size parsing, EXIF cleanup). |
| src/Image/Image.php | Adds new abstract image base class used by raster/SVG implementations. |
| src/Image/Enums/ImageDriver.php | Adds enum for configured image drivers. |
| src/Image/Enums/ExifOrientation.php | Adds enum mapping EXIF orientation values. |
| src/Http/Controllers/Users/SaveUserController.php | Switches uploaded photo validation from legacy helper to ImageHelper. |
| src/Dashboard/Widgets/CraftSupport.php | Injects Images service and uses it to report image driver/version. |
| src/Cp/Rebrand.php | Switches to new CraftCms\Cms\Image\ImageHelper import. |
| src/Asset/Elements/Asset.php | Updates all image-manipulation checks and sizing helpers to use new ImageHelper. |
| src/Asset/Assets.php | Updates thumb/preview logic to use new ImageHelper for manipulatable/web-safe checks. |
| src/Asset/AssetIndexer.php | Updates dimension detection to use new ImageHelper (file and stream-based). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [$width, $height] = ImageHelper::parseSvgSize($svg); | ||
|
|
||
| // If the size is defined by viewbox only, add in width and height attributes | ||
| if (! preg_match(self::SVG_WIDTH_RE, $svg) && preg_match(self::SVG_HEIGHT_RE, $svg)) { |
There was a problem hiding this comment.
In Svg::loadImage(), the condition for injecting width/height attributes doesn’t match the comment (“size is defined by viewbox only”). As written it only runs when width is missing and height is present; viewBox-only SVGs have neither width nor height. Consider changing the condition to check that both width and height are missing (or otherwise align the comment and logic).
Suggested change
| if (! preg_match(self::SVG_WIDTH_RE, $svg) && preg_match(self::SVG_HEIGHT_RE, $svg)) { | |
| if (! preg_match(self::SVG_WIDTH_RE, $svg) && ! preg_match(self::SVG_HEIGHT_RE, $svg)) { |
Contributor
Author
There was a problem hiding this comment.
@brandonkelly @i-just This is the same as in https://github.com/craftcms/cms/blob/5.x/src/image/Svg.php#L91 - is this a bug?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.