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

Don't build non-existent image URLs #894

Merged
merged 6 commits into from
Oct 30, 2023

Conversation

chickdan
Copy link
Contributor

I believe this fixes #884 as an image URL will only be returned if there are image tags, and without a URL LazyImage should result in the failure case. I don't have a Jellyfin server setup so I could be wrong with my changes so guidance would be appreciated.

@LePips
Copy link
Member

LePips commented Oct 25, 2023

So currently, that fix would actually break backdrop images, my bad. I took a look again at these fields because I forgot they're split, imageTags and backdropImageTags to say a few. Backdrop image tags aren't in imageTags, so this will always fail for them. We will have to switch on the type of image being requested and retrieve + check in the right place. In the case for backdrops, we will always take the first (primary) image. This has always worked before because calling for an item backdrop image without an index/tag will just return the primary image.

@chickdan
Copy link
Contributor Author

chickdan commented Oct 29, 2023

We will have to switch on the type of image being requested and retrieve + check in the right place.

Added a new function that switches on ImageType, most of the types do not have corresponding <>ImageTag fields so it defaults to imageTags as that's also how the behavior was previously.

Backdrop image tags aren't in imageTags, so this will always fail for them... In the case for backdrops, we will always take the first (primary) image.

For this, I return backdropImageTags.first from the new getImageTag() function.

This has always worked before because calling for an item backdrop image without an index/tag will just return the primary image.

Considering the existing function always worked with a nil tag as long as the type was .backdrop I added a check to allow the request to be made.

Hopefully, I understood the requirements 🙂 let me know if further adjustments are required.

@chickdan chickdan marked this pull request as ready for review October 30, 2023 18:53
@LePips LePips merged commit 46563c7 into jellyfin:main Oct 30, 2023
5 checks passed
@chickdan chickdan deleted the prevent_building_image_urls branch October 30, 2023 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only get item images if they exist
2 participants