-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
OcWeb: Improve OC corresponding JS method #2879
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
base: master
Are you sure you want to change the base?
Conversation
@@ -94,7 +113,7 @@ private val SCREEN_ID_TO_URL = hashMapOf( | |||
10007 to "https://myaccount.google.com/payments-and-subscriptions", | |||
10015 to "https://support.google.com/accounts", | |||
10050 to "https://myaccount.google.com/profile", | |||
10052 to "https://myaccount.google.com/embedded/family/create", | |||
10052 to "https://myaccount.google.com/u/1/family/details", |
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.
I'm not sure but doesn't /u/1/
mean it only go to the first account.
Does it works also for secondary accounts?
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.
Modified, thanks
private const val TAG = "JS_$NAME" | ||
} | ||
|
||
@OptIn(ExperimentalStdlibApi::class) |
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.
Please don't use experimental API.
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.
changed
fun getAndroidId(): String? { | ||
Log.d(TAG, "getAndroidId: ") | ||
val androidId = LastCheckinInfo.read(context).androidId | ||
return if (androidId != 0L) androidId.toHexString() else null |
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.
return if (androidId != 0L) androidId.toHexString() else null | |
return if (androidId != 0L) androidId.toString(16) else null |
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.
Changed
} | ||
|
||
@JavascriptInterface | ||
@Synchronized |
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.
Why all the @Synchronized
? JavaScript is already single-thread anyway.
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.
Changed
@JavascriptInterface | ||
fun getOsVersion(): String? { | ||
Log.d(TAG, "getOsVersion: ") | ||
return RELEASE |
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.
I think returning from the org.microg.gms.profile.Build
would be more appropriate here.
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.
Changed
|
||
else -> { | ||
pendingMimeType = mimeType | ||
cameraPermissionLauncher.launch(Manifest.permission.CAMERA) |
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.
Don't request the CAMERA permission before the user selected to upload a photo from camera, there is no reason to request and grant the permission if the user only wants to upload a file from storage.
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.
The idea here is that after the user chooses to take a photo, if there is no camera permission, it will actively apply for permission to allow the user to use the photo function. Modified, do not actively apply for camera permissions
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.
It would be fine to ask for camera permission after the user selected to use the camera. So user flow would be:
- User selects to set new avater
- User is presented options "Take picture" and "Choose from Gallery"
- User selects "Take picture"
- User is asked for Camera permission
- Camera is opened
If the user does not select "Take picture", there is no need to ask for Camera permission.
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.
Understand. Of course, it has been modified now to only let users choose to take pictures or albums when they have camera permissions. If they do not have permissions, they will only jump to the album. This can avoid users from participating in too many permission granting processes.
Supplement the JS method corresponding to OcIdWebview.
Also modify the following: