Skip to content
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

Uploading SVGs without easy-thumbnails[svg] leaves db in inconsistent state #1433

Closed
benzkji opened this issue Nov 1, 2023 · 13 comments
Closed
Labels

Comments

@benzkji
Copy link
Contributor

benzkji commented Nov 1, 2023

Without easy-thumbnails[svg] (i.e. reportlab) SVGs are still uploaded/stored as Image, but they have width=Null and height=Null stored. To be really consistent, one could store SVGs as File, when no support for that image format is available? Or deny SVG upload completly, without support?

Currently, it can be a pain, as one can start a project without knowing this, and ending up with a few hundred svgs in the db, without dimensions.

Dont know what the best solution could be...?

@benzkji
Copy link
Contributor Author

benzkji commented Nov 4, 2023

from django.core.management import BaseCommand
from django.db.models import Q
from easy_thumbnails.VIL import Image as VILImage
from filer.models import Image


class Command(BaseCommand):
    help = (
        "update dimensions of svg images that have no dimensions set, "
        "for whatever reason"
    )

    def handle(self, *args, **options):
        no_dimensions = Image.objects.filter(file__endswith=".svg").filter(
            Q(_width=0) | Q(_width__isnull=True)
        )
        self.stdout.write(f"trying to set dimensions on {no_dimensions.count()} files")
        for image in no_dimensions:
            self.stdout.write(image.path)
            try:
                imgfile = image.file.file
            except ValueError:
                imgfile = image.file_ptr.file
            imgfile.seek(0)
            image._width, image._height = VILImage.load(imgfile).size
            image.save()
        return

Quickhack, copied some code from filer. but I get a lot of these errors:
ERROR:root:Unsupported shape type Group for clipping

Dimensions are not updated. And it takes a very long time, for every single SVG. One can argue that my SVGs are too complex or whatever, but these are valid SVGs. I can provide examples if needed?

UPDATE: All above is true, except that it actually works! Dimensions are set. I've corrected my code, this could be used in filer in this or similar form...for people that forget easy-thumbnails[svg].

@fsbraun
Copy link
Member

fsbraun commented Nov 4, 2023

@benzkji Can I interest you in creating a PR with that code? I am not sure if that error throws an exception. Maybe just catch the output?

@benzkji
Copy link
Contributor Author

benzkji commented Nov 4, 2023

yes. master is the branch to branch off, right?

@benzkji
Copy link
Contributor Author

benzkji commented Nov 4, 2023

It doesnt throw an error, but logged 10'000 entries to my Sentry this morning. PR on the way.

@benzkji
Copy link
Contributor Author

benzkji commented Nov 4, 2023

#1434

@jrief
Copy link
Collaborator

jrief commented Nov 6, 2023

No, this is a django-filer issue and has to be addressed here.

@benzkji
Copy link
Contributor Author

benzkji commented Nov 6, 2023

@jrief have you seen the PR? Not sure which direction to go.

Also, fixing (with management command) is one thing, and preventing getting in an inconsistent state is another.

@fsbraun
Copy link
Member

fsbraun commented Nov 8, 2023

@benzkji Can we close this issue now that #1434 is merged?

@benzkji
Copy link
Contributor Author

benzkji commented Nov 8, 2023

not really sure...as #1434 only does help when things already went wrong...but prevent getting there would be the goal. I don't have a definitive answer, but tend to the "deny uploads" when SVG support is not available? If you decide to not do anything, I'm fine too, feel free to close then ;-)

@fsbraun
Copy link
Member

fsbraun commented Nov 8, 2023

Or maybe just turn a SVG into a File object as opposed to Image object if SVG support is not installed. That's probably easily achievable. If SVG support is added later, however, Files will remain Files.

@benzkji
Copy link
Contributor Author

benzkji commented Nov 8, 2023

Yes, but could lead to other undesired situations (client uploads a few SVGs, then has to re-upload and re-assign everything, when "no SVG support" is detected). I now know about, so...it's just about finding the most accessible solution for people who don't. I would prefer an Image withouth width, and being able to call the managemen command later on.

Copy link

stale bot commented Feb 6, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 6, 2024
Copy link

stale bot commented Mar 6, 2024

This will now be closed due to inactivity, but feel free to reopen it.

@stale stale bot closed this as completed Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants