cordova-android icon indicating copy to clipboard operation
cordova-android copied to clipboard

CordovaPlugin.hasPermisssion method name typo

Open shader opened this issue 6 years ago • 5 comments

https://github.com/apache/cordova-android/blob/8a4ae311ce165e31f0511ae43274aa52d3773fd2/framework/src/org/apache/cordova/CordovaPlugin.java#L407

Clearly, the name hasPermisssion is misspelled, and should be hasPermission instead...

shader avatar Nov 29 '18 19:11 shader

Renaming this is a major change that is guaranteed to break a bunch of existing plugins :(

dpogue avatar Nov 29 '18 20:11 dpogue

Yes, changing it suddenly would certainly break a lot of things.

Surely it would be possible to mark the API as 'deprecated', change it to an alias for the new correctly spelled method, and then eventually remove it in a future version?

shader avatar Nov 29 '18 20:11 shader

If the plugins were all maintained, and adapted to the new API, that would work, but our experience suggests that lots of plugins are not actively maintained.

We could potentially look into this in the next next major, since we're already discussing potential breakage to unmaintained plugins using older patterns.

dpogue avatar Nov 29 '18 20:11 dpogue

How hard would it be to make it so that older plugins can be built against a specific version of the plugin API? That would help, even if whoever is using the plugins has to explicitly configure it (since obviously the plugin itself will not).

shader avatar Nov 29 '18 20:11 shader

From a quick code search I find the misspelled hasPermisssion method is called in only one piece of code, in the following function: https://github.com/apache/cordova-android/blob/8a4ae311ce165e31f0511ae43274aa52d3773fd2/framework/src/org/apache/cordova/engine/SystemWebChromeClient.java#L170-L180

I think this would only be an issue for any plugins with feature name="Geolocation", and not many plugins other than cordova-plugin-geolocation would use this feature name.

I think of the following quick solution:

  • we introduce the correct method name ("hasPermission") in CordovaPlugin.java
  • CordovaPlugin.java tries both "hasPermission" and misspelled hasPermisssion
  • Update cordova-plugin-geolocation to override the new method, in a major version increase, with dependency on new cordova-android engine version
  • Mark the incorrect hasPermisssion method as deprecated, for complete removal at some point in the future (with plenty of warning of course)

Ideas for refinement:

  • new callback method should be void onGeolocationPermissions or onGeolocationPermissionsShowPrompt hook function, which the plugin would override to call its own requestPermissions function if necessary
  • make it possible for multiple plugins to override the new callback, and Cordova would query all plugins, like it does on onPause, onReceivedHttpAuthRequest, etc.

brody4hire avatar Nov 29 '18 22:11 brody4hire