devicon icon indicating copy to clipboard operation
devicon copied to clipboard

[FEATURE REQUEST] Automatically check if SVGs contain raster images

Open Snailedlt opened this issue 2 years ago • 6 comments

I have searched through the issues and didn't find my problem.

  • [X] Confirm

Problem

Currently raster images are allowed in the SVG's by the check-bot. This should not be the case, since raster images are not scalable, and therefore don't belong in our icons.

Possible Solution

Add checks to check-bot. The bot should check for <img> and <image> tags

Additional information

See #1517 for some examples and more info

Snailedlt avatar Nov 20 '22 21:11 Snailedlt

Hello there! Perhaps, we can use this regular expression.

/="data:image\/.+;base64,/gm

Well, let me try to convert it to Python: 😅

import re

# A mockup of a SVG.
svg_body = "<svg>...</svg>"

# Keeps the amount of images/raster founded.
is_img = len(re.findall("=\"data:image\/.+;base64,", svg_body))

if is_img > 0:
  err_msg.append(f"- `{svg_path.name}` has an image/raster within it.")

This is my first time ever coding in that programming language.

lunatic-fox avatar Nov 29 '22 03:11 lunatic-fox

@lunatic-fox no need to re-invent the wheel here. I think we can just re-use what SVGO has here: https://github.com/svg/svgo/blob/ae32acf5144be318872a45f49b3ece50c0a4cb18/plugins/removeRasterImages.js#L24

Snailedlt avatar Nov 29 '22 17:11 Snailedlt

Hmm, the idea came from here. They're regex is a step before and it will totally work.

The full regular expression with common image extensions would be something like this:

/<image.+(xlink:)?href="data:image\/(png|jpg|jpeg|gif|tiff|svg\+xml|bmp);base64,.+"\/>/gm

But since there are more extensions we could reduce to this:

/<image.+(xlink:)?href="data:image\/.+;base64,.+"\/>/gm

Or to simplify everything we can just ask if the svg has the <image> tag, that may be more than enough to know if there's a image/raster. So we can reuse line 22 instead. 🙂

lunatic-fox avatar Nov 29 '22 18:11 lunatic-fox

Yeah, seems like a good idea to just generally not allow the image element. That way we disallow raster images and other linked images, so we have the entire source code in our repo instead of linking to external images.

Snailedlt avatar Nov 29 '22 19:11 Snailedlt

Code input n1

check_icon_pr.py

# ...
def check_svgs(svg_file_paths: List[Path]):
    err_msgs = []
    for svg_path in svg_file_paths:
        try:
            err_msg = [f"SVG Error in '{svg_path.name}':"]

            # name check

            # ...

            # svg check

            # ...

            # check for image/raster inside the svg
            icon = open(svg_path)
            has_image = icon.read().find('image') > -1 or False
            icon.close()
            if has_image:
                err_msg.append(f'The icon "{svg_path.name}" has an image within.')
            
            # ...
# ...

Isolated code

# check for image/raster inside the svg
icon = open(svg_path)
has_image = icon.read().find('image') > -1 or False
icon.close()
if has_image:
    err_msg.append(f'The icon "{svg_path.name}" has an image within.')

I've decided to make this checker as simple as possible, but if we want it to be more specific it's possible to use a Regular Expression, instead of find. That way we can even know how many images are inside the svg icon.

lunatic-fox avatar Dec 05 '22 16:12 lunatic-fox

@lunatic-fox The code looks great! Nice work!

We should make the error message a big clearer though. Maybe like this? The icon "{svg_path.name}" contains an <image> element. This is usually a raster image, and therefore needs to be removed or replaced by another element

Snailedlt avatar Feb 18 '23 16:02 Snailedlt