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

Improve JS injection method #439

Merged

Conversation

CarlosOlivo
Copy link
Contributor

No more undefined NativeShell

Before

chrome_1FT2lPrgak

V/WebView: ApiClient serverAddress: http://10.0.2.2:8096, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js (2)
V/WebView: ApiClient appName: undefined, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js (2)
V/WebView: ApiClient appVersion: undefined, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js (2)
V/WebView: ApiClient deviceName: undefined, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js (2)
V/WebView: ApiClient deviceId: undefined, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js (2)

After

chrome_xPapohG59v

V/WebView: ApiClient serverAddress: http://10.0.2.2:8096, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js?1623960060466 (2)
V/WebView: ApiClient appName: Jellyfin Android, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js?1623960060466 (2)
V/WebView: ApiClient appVersion: 0.0.0-dev.1, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js?1623960060466 (2)
V/WebView: ApiClient deviceName: sdk_gphone_x86, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js?1623960060466 (2)
V/WebView: ApiClient deviceId: 9c17977ed5b293fb, http://10.0.2.2:8096/web/main.4b4b736dbefe7c3a90ec.bundle.js?1623960060466 (2)

Maxr1998
Maxr1998 previously approved these changes Jun 18, 2021
Copy link
Member

@Maxr1998 Maxr1998 left a comment

Choose a reason for hiding this comment

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

Looks good! I will do some more on-device testing tomorrow and merge it if I don't find any side-effects.

@CarlosOlivo
Copy link
Contributor Author

I found that WebViewAssetLoader.AssetsPathHandler exists, so I removed WebAppUtils.kt

@Maxr1998 Maxr1998 added the enhancement New feature or request label Jun 18, 2021
Co-authored-by: Niels van Velzen <[email protected]>
@CarlosOlivo CarlosOlivo force-pushed the improve-js-injection-method branch from d334116 to 5bcd2d6 Compare June 19, 2021 07:55
nielsvanvelzen
nielsvanvelzen previously approved these changes Jun 19, 2021
@nielsvanvelzen nielsvanvelzen added the backport Pull request needs to be backported to current release branch label Jun 19, 2021
Maxr1998 added 3 commits June 19, 2021 13:42
The main bundle is in the body too, so it makes sense to keep it like that.
Maxr1998
Maxr1998 previously approved these changes Jun 19, 2021
@Maxr1998 Maxr1998 requested a review from nielsvanvelzen June 19, 2021 14:52
@CarlosOlivo CarlosOlivo requested a review from Maxr1998 June 19, 2021 16:00
@CarlosOlivo
Copy link
Contributor Author

See #439 (comment) I think it is better than running a regex comparison for each request.

@nielsvanvelzen nielsvanvelzen merged commit 249bde9 into jellyfin:master Jun 19, 2021
@CarlosOlivo CarlosOlivo deleted the improve-js-injection-method branch June 19, 2021 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Pull request needs to be backported to current release branch enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants