server
server copied to clipboard
bitwarden
[PS-1280] small changes to icon fetching for self-hosted services
Type of Change
Bug Fix
Objective
The problematic website in question is Jellyfin. There were several issues preventing Bitwarden from retrieving a proper icon so I made changes to the relevent sections, but let me know if some of this needs to be removed.
Code Changes
Priority: It seems like an icon link with no sizes property will get ranked higher than one with sizes="256x256" even though the latter has a confirmed size. I'm honestly not sure if this is true in practice, since the apple-touch-icon link was selected over the apple-touch-startup-image links for Jellyfin, despite the startup images having no size listed.
Base URL: This was the main issue and the only important change in this pull request. Bitwarden is checking for the <base> element which doesn't normally exist for self-hosted services. Jellyfin will send a 302 redirect to /web/index.html which was properly handled, but the resulting links were using / as the base URL which caused every single icon to fail. Not sure what the best solution is, but this pull request contains the quick solution I applied during testing.
Filtering: The favicon was ignored for Jellyfin only because it was near the end of the links in the head block and got filtered out by icons.OrderBy(i => i.Priority).Take(10) during the process. Not sure how helpful this will be in the future but I figured I would bring attention to the problem.
Testing Requirements
Jellyfin has a demo instance you can use to see this bug in action.
Hi @dkanada , thank you for submitting this PR, and apologies for delay in reaching out. This will be added to our internal community PR for review, and we will get back to you with our feedback and/or concerns.
Do you know if the change from 10 to 20 will solve the problem, or is that an arbitrary number? Did changing the baseUrl from "/" have any use?
It has been a while since I tested this, but I believe both were important for the icon. The base URL was causing issues because Jellyfin serves the favicon at /web and the smaller limit was filtering out the favicon due to a large amount of unrelated results.
To better answer the question, I did indeed choose 20 because it passed the threshold for the Jellyfin favicon, and it seemed like a sane default for other sites as well.