-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Check WebView Version For Login Functionality #12108
Check WebView Version For Login Functionality #12108
Conversation
Signed-off-by: alperozturk <[email protected]>
Signed-off-by: alperozturk <[email protected]>
* | ||
*/ | ||
private fun getMinimumSupportedMajorWebViewVersion(): String { | ||
return "118" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AndyScherzinger @tobiasKaminsky Can we get that value from backend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server doesn't have/know this. So we could for the moment hard-code this on client-side and bump it per major release of the server.
Signed-off-by: alperozturk <[email protected]>
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12108.apk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea! The design LGTM :)
dialog.show() | ||
} | ||
|
||
private fun redirectToAndroidSystemWebViewStorePage() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A very similar function already exists in DrawerActivity.java
, called openAppStore()
. Currently its visibility is set to private. Would it be possible to consolidate this functionality?
val pm: PackageManager = context.packageManager | ||
|
||
return try { | ||
val pi = pm.getPackageInfo("com.google.android.webview", 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During my tests, this line consistently throws PackageManager.NameNotFoundException
. This doesn't work as expected on devices running AOSP. Maybe the package name differs on AOSP images? It's called com.android.webview
on my phone running LineageOS.
I've been able to reproduce this using the following configurations:
- Pixel 2 (Android Studio Emulator) x86_64 API 26 AOSP
- Pixel 2 (Android Studio Emulator) x86_64 API 24 AOSP
- Pixel 2 (Android Studio Emulator) x86_64 API 30 AOSP
- OnePlus 6T API 33 LineageOS 20
In that case, one would also have to consider third party SystemWebView implementations, such as Bromite, for example.
However, because the calling functions returns true
, if this function returns null
, this could possibly be ignored?
Problem:
Login feature relies on WebView. If Android System WebView version is outdated login functionality is not working. Therefore we need to inform the user to update Android System WebView.
If web view version outdated:
Screen_Recording_20231030_113148_Google.Play.Store.mp4
@nextcloud/designers Is it suitable in terms of UI/UX?