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

Fix trailing slash when adding server causes connection faliure. #5400

Closed
wants to merge 1 commit into from

Conversation

vcangi
Copy link

@vcangi vcangi commented Apr 20, 2024

Addresses a bug where a trailing "/" will prevent a connection to a server.

Initial issues from jellyfin-media-player
Corresponding PR found here.

Copy link

sonarcloud bot commented Apr 20, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit 1ce3c6a
Status ✅ Deployed!
Preview URL https://a8c6c794.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs
View bot logs

Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, this error occurs when the server has already been added to the list (before the add server page was fixed in #5205), right?

@@ -105,6 +105,7 @@ function showServerConnectionFailure() {
export default function (view, params) {
function connectToServer(server) {
loading.show();
server = server.replace("/$", "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the add server page, this is done as follows:

const host = page.querySelector('#txtServerHost').value.replace(/\/+$/, '');

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I re-ran some of my tests and it looks like it's specifically an issue with the media-player, not in here.

I'll close this and look into it there.

Thanks!

@vcangi vcangi closed this Apr 20, 2024
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.

None yet

3 participants