You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2022/08/02 14:12:35 UTC

[GitHub] [cordova-plugin-geolocation] dpa99c opened a new pull request, #250: (android) fix: respect requested location accuracy on Android 12+

dpa99c opened a new pull request, #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250

   ### Platforms affected
   Android
   
   
   
   ### Motivation and Context
   <!-- Why is this change required? What problem does it solve? -->
   <!-- If it fixes an open issue, please link to the issue here. -->
   Prior to Android 12, both COARSE and FINE location permissions were treated synonymously - requesting one would grant the other.
   Therefore this plugin currently checks/requests both permissions on Android.
   
   However, when an app is built with an SDK version (and running on device with) Android 12 (API 31) or higher, Android differentiates between precise vs approximate accuracy using these permissions (similar to iOS 14+) - see the [Android documentation](https://developer.android.com/training/location/permissions#approximate-request).
   So currently when the methods of this plugin are invoked on Android 12+, this results in both both COARSE and FINE location permissions being requested, regardless of the value set for `enableHighAccuracy` in `PositionOptions`, resulting in the user being asked to choose between precise vs approximate location permission:
   
   <img width="300" src="https://user-images.githubusercontent.com/2345062/182159796-7ebc7014-bc4e-42ad-ac23-54d0fea4f9f1.png" />
   
   
   
   ### Description
   <!-- Describe your changes in detail -->
   This PR changes the Android implementation to respect the value of `enableHighAccuracy` passed in `PositionOptions` so if `enableHighAccuracy: false` then only COARSE location permission is requested, resulting in the user only being asked to grant approximate location permission:
   
   <img width="300" src="https://user-images.githubusercontent.com/2345062/182160379-2e926b06-80fb-4bc4-baee-de857e7e7b0c.png" />
   
   If the user grants this permission and `enableHighAccuracy: true` is subsequently requested, the user in prompted to grant an upgrade from approximate to precise location permission:
   
   <img width="300" src="https://user-images.githubusercontent.com/2345062/182394458-0777a59c-56cb-4ea1-8ac6-2f747eaea2e0.png" />
   
   If the app's first permission request is for `enableHighAccuracy: true`, then the user will be presented with the choice between approximate and precise location:
   
   <img width="300" src="https://user-images.githubusercontent.com/2345062/182161559-c0b8da37-6262-4849-9df4-419e513b3432.png" />
   
   I believe this PR resolves issues #188 and #235.
   
   One further change this PR makes is to remove the check for location permissions on Android before invoking `clearWatch()` since this is unnecessary.
   
   #### Issues with approximate location permission on Android
   
   Note: As @jcesarmobile points out [in this comment](https://github.com/apache/cordova-plugin-geolocation/issues/235#issuecomment-984536156), there is a [bug in the Chromium WebView](https://bugs.chromium.org/p/chromium/issues/detail?id=1269362) which is present up until API 32 (empirical testing shows it to manifest in every API from 22 to 31 but not in 32+ where it appears to be fixed).
    
   It happens when COARSE but not FINE location permission is granted and `enableHighAccuracy: false` is requested, and results in a `{"code":3,"message":"Timeout"}` error when `getCurrentLocation()` or `watchPosition()` is called, rather than returning the current location.
   Due to this issue, this PR implements API version-conditional functionality so that even when `enableHighAccuracy: false` is specified, both COARSE and FINE location permissions will still requested on API 31 and below, while on API 32 and above only COARSE permission will be requested.
   
   A further complication is specific to devices running API 31: even if the user grants precise (both COARSE and FINE) location permissions, if  `enableHighAccuracy: false` is passed in the `PositionOptions` object to the W3C Geolocation API function on the original `navigator.geolocation` by this plugin, then this also results in a `{"code":3,"message":"Timeout"}` error.
   To prevent this, this PR modifies the native `getPermission` method on Android to return the current API level of the device.
   The web layer of the plugin `www/android/geolocation.js` then checks this returned value and if it's equal to 31, ensures `enableHighAccuracy: true` is passed to to the W3C Geolocation API function, even if `enableHighAccuracy: false` was specified by the app.
   
   
   There is one further edge case: on Android 12+ (API 32+), if `enableHighAccuracy: true` is requested but the user grants only approximate (COARSE but not FINE) location permission, this will result in a `{"code":1,"message":"Illegal Access"}` error.
   This is best handled by the app informing the user that precise location permission is required but they only granted approximate location permission.
   The [test harness app](https://github.com/dpa99c/cordova-plugin-geolocation-test) I created for this PR demonstrates this.
   
   ### Testing
   <!-- Please describe in detail how you tested your changes. -->
   
   - Ran the tests in `cordova-paramedic` with the same results between current plugin master branch and PR branch:
   <img width="300" src="https://user-images.githubusercontent.com/2345062/182163034-10f09140-75f0-4ee1-989c-16294dcb85cc.png" />
   
   - Created a test harness app to test the current vs modified plugin behaviour: 
   	- https://github.com/dpa99c/cordova-plugin-geolocation-test
   	- Tested on Android emulators with APIs: 22, 29, 30, 31, 32, 33
   
   
   
   To run the test harness app with this PR branch of the plugin:
   ```
   git clone https://github.com/dpa99c/cordova-plugin-geolocation-test && cd cordova-plugin-geolocation-test
   cordova platform add android@latest
   cordova run android
   ```
   
   To run the test harness app with the `apache/master` branch of the plugin:
   ```
   git clone https://github.com/dpa99c/cordova-plugin-geolocation-test && cd cordova-plugin-geolocation-test
   git checkout apache_master
   cordova platform add android@latest
   cordova run android
   ```
   
   ### Checklist
   
   - [x] I've run the tests to see all new and existing tests pass
   - [x] I added automated test coverage as appropriate for this change (NOT APPLICABLE)
   - [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
   - [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
   - [x] I've updated the documentation if necessary (NOT APPLICABLE)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] dpa99c commented on a diff in pull request #250: (android) fix: respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
dpa99c commented on code in PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#discussion_r997966517


##########
www/android/geolocation.js:
##########
@@ -29,7 +29,12 @@ var pluginToNativeWatchMap = {};
 
 module.exports = {
     getCurrentPosition: function (success, error, args) {
-        var win = function () {
+        var win = function (deviceApiLevel) {
+            // Workaround for bug specific to API 31 where requesting `enableHighAccuracy: false` results in TIMEOUT error.
+            if (deviceApiLevel === 31) {

Review Comment:
   This is a bug specific to API 31 (<31 there was no distinguishing approx vs precise location, >=32 it has been fixed) - see above PR description for details



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] gohrms commented on pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
gohrms commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1327719666

   @js-kulkarni , @mobiliseapplabllp I am also facing the same issue. Since few days suddenly it stopped working on few mobile phone and in few others it is working properly. Tried so damn hard to get it working. Lets stay connected if someone finds a solutions then lets update here. Thank you. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] dpa99c commented on pull request #250: (android) fix: respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
dpa99c commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1282102734

   @breautek I've made the requested changes - can you re-review and confirm you are happy to merge?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] expcapitaldev commented on a diff in pull request #250: (android) fix: respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
expcapitaldev commented on code in PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#discussion_r997970433


##########
www/android/geolocation.js:
##########
@@ -29,7 +29,12 @@ var pluginToNativeWatchMap = {};
 
 module.exports = {
     getCurrentPosition: function (success, error, args) {
-        var win = function () {
+        var win = function (deviceApiLevel) {
+            // Workaround for bug specific to API 31 where requesting `enableHighAccuracy: false` results in TIMEOUT error.
+            if (deviceApiLevel === 31) {

Review Comment:
   thanks a lot I forgot



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] gohrms commented on pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
gohrms commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1332398428

   Guys, Any update pls ? Any help for this topic will be appreciated. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] KishanVaishnani commented on pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
KishanVaishnani commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1333293133

   I am facing the same issues in VIVO, OPPO, and MI
   
   I have enabled `enableHighAccuracy: true` and it will ask for 1 popup with both permissions If I choose `Approximate` then the code breaks. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] gohrms commented on pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
gohrms commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1333328767

   Good news, seems like Android have some update on Play Service which got download on VivoPhone automatically. I did restart my phone and the GPS is working fine now which earlier was not working on Vivo. 
   Can anyone please confirm on their phone, where GPS was not working.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] dpa99c merged pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
dpa99c merged PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] bhandaribhumin commented on pull request #250: (android) fix: respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
bhandaribhumin commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1256150580

   When do you plan to merge these changes?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] mobiliseapplabllp commented on pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
mobiliseapplabllp commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1324605306

   My geolocation plugin working fine before 2 day suddenly my one plus nord and many client compalnt application location not detect. it was previously working fine 
   
   "cordova-plugin-geolocation": "^4.1.0",
   "@ionic-native/geolocation": "^5.36.0",


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] expcapitaldev commented on pull request #250: (android) fix: respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
expcapitaldev commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1277384544

   @dpa99c can you check your MR ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] expcapitaldev commented on a diff in pull request #250: (android) fix: respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
expcapitaldev commented on code in PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#discussion_r997961880


##########
www/android/geolocation.js:
##########
@@ -29,7 +29,12 @@ var pluginToNativeWatchMap = {};
 
 module.exports = {
     getCurrentPosition: function (success, error, args) {
-        var win = function () {
+        var win = function (deviceApiLevel) {
+            // Workaround for bug specific to API 31 where requesting `enableHighAccuracy: false` results in TIMEOUT error.
+            if (deviceApiLevel === 31) {

Review Comment:
   maybe if deviceApiLevel > 30 ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] expcapitaldev commented on pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
expcapitaldev commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1282404806

   thanks a lot 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] js-kulkarni commented on pull request #250: fix(android): respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
js-kulkarni commented on PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#issuecomment-1327052484

   All of a sudden "cordova-plugin-geolocation" stopped working on some models of VIVO, OPPO, and some MI mobile phones from 17 Nov 2022. When can we expect a quick fix?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-plugin-geolocation] expcapitaldev commented on a diff in pull request #250: (android) fix: respect requested location accuracy on Android 12+

Posted by GitBox <gi...@apache.org>.
expcapitaldev commented on code in PR #250:
URL: https://github.com/apache/cordova-plugin-geolocation/pull/250#discussion_r997961880


##########
www/android/geolocation.js:
##########
@@ -29,7 +29,12 @@ var pluginToNativeWatchMap = {};
 
 module.exports = {
     getCurrentPosition: function (success, error, args) {
-        var win = function () {
+        var win = function (deviceApiLevel) {
+            // Workaround for bug specific to API 31 where requesting `enableHighAccuracy: false` results in TIMEOUT error.
+            if (deviceApiLevel === 31) {

Review Comment:
   maybe if deviceApiLevel > 31 ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org