Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Validator] Validate SVG ratio in Image validator #59265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 29, 2024

Conversation

maximecolin
Copy link
Contributor

@maximecolin maximecolin commented Dec 19, 2024

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #45269
License MIT

Implement ratio check for SVG images. Checking SGV size is not relevant as a SVG image can be enlarged without loss, but ratio can be important to check.

Currently, the validator add a violation with SIZE_NOT_DETECTED_ERROR in case of SVG image.

SVG size is guessed from viewbox, width and height attributes. Viewbox will provides default size, width and height can override viewbox size if they are number. Width and height as percentage are ignored as the final size will depend on the container.

I use preg_match instead of \DomDocument or simplexml functions to extract viewBox, width and height in order to avoid new dependencies on ext-dom or ext-simplexml.

In case of SVG, minWidth, maxWidth, minHeight, maxHeight, minPixels and maxPixels are ignored because not relevant. Only maxRatio, minRatio, allowSquare, allowLandscape and allowPortrait can generate violations, like suggested in the comments of the abandoned #45486.

@carsonbot carsonbot added this to the 7.3 milestone Dec 19, 2024
@maximecolin maximecolin force-pushed the valator-image-ratio-svg branch 4 times, most recently from 5f2304e to 9e7ecc1 Compare December 19, 2024 14:58
@maximecolin
Copy link
Contributor Author

Thanks @stof, I fixed the code according to your comments.

@maximecolin maximecolin force-pushed the valator-image-ratio-svg branch 2 times, most recently from 9281d5c to 4b270ff Compare December 19, 2024 15:46
@maximecolin maximecolin requested a review from stof December 19, 2024 15:46
@maximecolin maximecolin force-pushed the valator-image-ratio-svg branch 2 times, most recently from 8726444 to 1c16abf Compare December 23, 2024 08:33
@maximecolin
Copy link
Contributor Author

Thanks @OskarStark, I applied your suggestions.

7.3
---

* Allow `Image` constraint to check SVG ratio
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Allow `Image` constraint to check SVG ratio
* Add support for ratio checks for SVG files to the `Image`

@fabpot fabpot force-pushed the valator-image-ratio-svg branch from 1c16abf to c4ad092 Compare December 29, 2024 13:01
@fabpot
Copy link
Member

fabpot commented Dec 29, 2024

Thank you @maximecolin.

@fabpot fabpot merged commit b013b62 into symfony:7.3 Dec 29, 2024
5 of 11 checks passed
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Jan 2, 2025
This PR was merged into the 6.4 branch.

Discussion
----------

[Validator] Cannot validate svg image size

As an addition of #20496 and considering what is described in related [code PR](symfony/symfony#59265) I think it can be helpful to explicit mention this restriction for SVG images

Commits
-------

248d4ff cannot validate svg image size
@fabpot fabpot mentioned this pull request May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Validator] Feat: Support SVGs when validating image dimensions
6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.