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

THUMBNAIL_NAMER setting does not have any effect when using filer #1093

Closed
benzkji opened this issue Aug 28, 2018 · 14 comments
Closed

THUMBNAIL_NAMER setting does not have any effect when using filer #1093

benzkji opened this issue Aug 28, 2018 · 14 comments
Labels

Comments

@benzkji
Copy link
Contributor

benzkji commented Aug 28, 2018

When dealing with SEO (SmileyChris/easy-thumbnails#474) or when one just wants a different THUMBNAIL_NAMER (SmileyChris/easy-thumbnails#492), there seems to be no luck with the filer.

There is the ThumbnailerNameMixin, that has it's own get_thumbnail_name() method, where the naming is fixed, as said there "A version of Thumbnailer.get_thumbnail_name that produces a
reproducible thumbnail name that can be converted back to the original
filename. : https://github.com/divio/django-filer/blob/006a1cfbdb5f61b6725b7675c7f9117e85b5d367/filer/utils/filer_easy_thumbnails.py#L32 That mixin is used for FilerThumbnailer, that is then created here: https://github.com/divio/django-filer/blob/006a1cfbdb5f61b6725b7675c7f9117e85b5d367/filer/models/abstract.py#L175 . I do not really get how this all works and interacts with easy_thumbnails. But the question remains: Why does filer need to override the namer, and get back to the original from the thumbnail? Normally, it would be enough to get the thumbnails, when for example deleting a file, so no orphaned thumbs are around?

And finally, am I missing something in the docs, and setting a custom namer is already possible?

@benzkji
Copy link
Contributor Author

benzkji commented Aug 28, 2018

Ah. I guess getting to the original is needed when verifying access thumbnails of secured files/images?! Making a custom namer possible on non-protected files would be a nice one, then :)

@benzkji
Copy link
Contributor Author

benzkji commented Aug 29, 2018

It is for serving protected thumbnails. As far as I can see, it could be changed to support custom namer, needing to have a setting for the regex.

RE_ORIGINAL_FILENAME = re.compile(r"^(?P<source_filename>.*)__(?P<opts_and_ext>.*?)$")

could be

RE_ORIGINAL_FILENAME = re.compile(r"^(?P<source_filename>.*).(?P<alias>.*?)$")

It would then be in the users responsibility, to make finding the original file work, when serving protected thumbnails. Or, as an alternative, only use the filer namer when creating protected files. This is not preffered, as technically "protected" files still can be public.

Feedback welcome!

@benzkji
Copy link
Contributor Author

benzkji commented Aug 29, 2018

"Hashed only" namers would not work, but still...the alias namer is crucial for image SEO, and a real drawback if not available.

@benzkji
Copy link
Contributor Author

benzkji commented Aug 30, 2018

a quick feedback from the team would be nice, like "we wont ever support" or "sure, wait, we'll do that right now, it's so easy".

thanks in advance @yakky @stefanfoulis ?

@ianare
Copy link

ianare commented Mar 11, 2019

In order to work around this issue I've configured my nginx server to redirect all resized images to the original, but only on search engine user agent.

In this way Google always gets the full sized image, and I don't have to worry about changing image dimensions in thumbnails.

Not saying this is ideal... but seems to work well enough.

@benzkji
Copy link
Contributor Author

benzkji commented Mar 11, 2019

thanks for the insight. least but ideal - you'll get really bad scores in page speed insights, this way...this being only the first thing coming to my mind ...

@ianare
Copy link

ianare commented Apr 15, 2019

I'm getting good scores on page speed insights (96 on dekstop / 54 on mobile), the bad score on mobile is due to "Defer offscreen images" and "Eliminate render-blocking resources" which we are in the process of fixing. PageSpeed uses a mobile Chrome user agant so is not impacted by this.

Here is my nginx mapping if of some interest to others:

#
# Search engines
#
map $http_user_agent $search_engine_agent {
    default 0;

    "~Applebot" 1;
    "~AhrefsBot" 1;
    "~Google" 1;
    "~[Bb]ingbot" 1;
    "~Slurp" 1;
    "~Crawlera" 1;
    "~Curious George" 1;
    "~LinkedInBot" 1;
    "~DuckDuckBot" 1;
    "~Baiduspider" 1;
    "~YandexBot" 1;
    "~LinkedInBot" 1;
    "~Sogou" 1;
    "~[Ee]xabot" 1;
    "~facebot" 1;
    "~ia_archiver" 1;
    "~Gigabot" 1;
    "~Gigablast" 1;
    "~Twitterbot" 1;
    "~OrangeBot" 1;
    "~MJ12bot" 1;
    "~SemrushBot" 1;
    "~SkypeUriPreview" 1;
}

and then it's used as follows in the virtual host conf file:

# search engines always get the high-resolution image
if ($search_engine_agent) {
    rewrite ^/media/filer_public_thumbnails/filer_public/(.+)__.+$ /media/filer_public/$1 permanent;
}

@benzkji
Copy link
Contributor Author

benzkji commented Apr 5, 2022

how about digging this one out - @jrief ? I hate monkey patching ;-)

@stale
Copy link

stale bot commented Jul 28, 2022

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 Jul 28, 2022
@stale
Copy link

stale bot commented Aug 31, 2022

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

@stale stale bot closed this as completed Aug 31, 2022
@benzkji
Copy link
Contributor Author

benzkji commented Aug 31, 2022

for me, this is an important issue. please re-open. I would even think about creating a PR, since they are reviewed more efficiently now...

@fsbraun fsbraun reopened this Aug 31, 2022
@stale stale bot removed the stale label Aug 31, 2022
@fsbraun
Copy link
Member

fsbraun commented Aug 31, 2022

@benzkji Thanks for keeping this alive! It would be wonderful if you could create a PR!

@stale
Copy link

stale bot commented Nov 29, 2022

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 Nov 29, 2022
@stale
Copy link

stale bot commented Dec 27, 2022

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

@stale stale bot closed this as completed Dec 27, 2022
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