Skip to content

Conversation

@GitToTheHub
Copy link
Contributor

Platforms affected

Android

Motivation and Context

  • Make clear, that only onRequestPermissionResult is currently save to use
  • Document, that onRequestPermissionResult is called from CordovaActivity after called from the system
  • Add document link for Android method onRequestPermissionsResult

Testing

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • 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)
  • I've updated the documentation if necessary

- Method is called after onRequestPermissionsResult was called by the system from CordovaActivity
- Add documentation link for Android method onRequestPermissionsResult
- Document, that the second method onRequestPermissionsResult is not save to use
@GitToTheHub
Copy link
Contributor Author

This PR is wrong sorry, because I was confused about these things:

  • CordovaActivity.onRequestPermissionsResult calls CordovaInterfaceImpl.onRequestPermissionResult without an s. CordovaInterfaceImpl.onRequestPermissionResult should be renamed to CordovaInterfaceImpl.onRequestPermissionsResult
  • CordovaInterfaceImpl.onRequestPermissionResult calls the deprecated method CordovaPlugin.onRequestPermissionResult which should be corrected

@GitToTheHub GitToTheHub changed the title Document onRequestPermissionResult Document onRequestPermissionResult (wrong pr) Jul 9, 2025
@GitToTheHub GitToTheHub changed the title Document onRequestPermissionResult (wrong pr) Document onRequestPermissionResult (fault pr) Jul 9, 2025
@GitToTheHub GitToTheHub changed the title Document onRequestPermissionResult (fault pr) Document onRequestPermissionResult (faulty pr) Jul 9, 2025
@erisu
Copy link
Member

erisu commented Oct 27, 2025

@GitToTheHub Does faulty pr mean we can close this?

@GitToTheHub
Copy link
Contributor Author

This was only to indicate, that I did something wrong but want to correct it. I will correct it now :)

@GitToTheHub GitToTheHub changed the title Document onRequestPermissionResult (faulty pr) Document onRequestPermissionResult (need clarification) Oct 27, 2025
@GitToTheHub
Copy link
Contributor Author

GitToTheHub commented Oct 27, 2025

Hi @erisu,

I was marking this pr before as "faulty" because I thought I mixed something up. But when I researched the Code to document the deprecated CordovaPlugin.onRequestPermissionResult and the new one CordovaPlugin.onRequestPermissionsResult, I saw, that the new one is never called by CordovaActivity. It would only be called by PermissionHelper, but this class nowhere used in the code. There is also an issue for this: #1388

So the following things are to clear:

  • Can the class PermissionHelper be removed? Because it isn't used anywhere
  • I would add a call to the new CordovaPlugin.onRequestPermissionsResult, where the old one was called. So both are called.
  • Rename CordovaInterfaceImpl.onRequestPermissionResult to CordovaInterfaceImpl.onRequestPermissionsResult

@GitToTheHub
Copy link
Contributor Author

I created two PRs to clear these things: #1855 and #1856

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants