search

jellyfin / jellyfin-android

Android Client for Jellyfin
https://jellyfin.org
GNU General Public License v2.0
1.33k stars 223 forks source link

Fix broken external links #1395

Open AllegraCodes opened 1 month ago

AllegraCodes commented 1 month ago

Changes Links in metadata would open in the app and lock it up. I override the function shouldOverrideUrlLoading to launch external links with the system. External links are defined as those that aren't intercepted by shouldInterceptRequest and don't contain the server hostname.

Issues Fixes #984

Maxr1998 commented 1 month ago

Hi, thanks for the PR. Do you have a specific example where this happens? For me, external links open in the browser just fine. So this seems to be an issue in the web client for some links instead.

AllegraCodes commented 1 month ago

Sure, I replicated the issue by editing the metadata of one of my items to have <a href="https://developer.android.com/guide/components/intents-filters#extras">my test link</a> in the Overview. Then I navigated to that item's page in the app and tapped the link, and it opened in the app, unresponsive to back gestures. Screenshot_20240522-162040

Maxr1998 commented 1 month ago

Fascinating. What type of library, item and which metadata field was it?

AllegraCodes commented 1 month ago

I always used the Overview field and the behavior is the same for a movie, show, playlist, or collection

Maxr1998 commented 1 month ago

I see, I can reproduce it now, thanks for providing all these details.

I'm worried that the current matchers aren't enough and would cause pages to open externally which shouldn't. Authentication components/plugins, for example. I feel like web should preferably handle this, but I'm not sure how.

AllegraCodes commented 1 month ago

that's a good point, i'll keep looking into this to see how android handles these calls and try to make it more robust.

Maxr1998 commented 1 month ago

Thanks! Appreciate your work, hopefully you can find a good solution to this issue.

AllegraCodes commented 1 month ago

ok, so the docs offer some notes on when this method actually gets called, and it's fairly limited. It's called for navigations initiated by the web page, the user, or an HTTP redirect.

Notably, it's not called for any navigations initiated by the app, but that doesn't mean it's impossible to trigger it from the app's code. The WebViewFragment triggers it when it loads \<serverUrl>/web on initialization (but not when it loads a javascript url from the lifecycle scope). This is allowed to load internally because it's on the same host (including port) as the server, and it's the only instance I found of the app itself triggering this method in my limited testing so far.

One way I could lock it down a bit further is to require the request be associated with a gesture (such as a click, but we only get a boolean, not the type) in order to load it externally. I think I'll add that now so any reviewers can see it.

With all that said, I'm an android and web app novice, so I could easily be missing something with how android works here.