Skip to content
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

[Fix] Permissions returned by onRequestPermissionResult is empty #2180

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

jinliu9508
Copy link
Contributor

@jinliu9508 jinliu9508 commented Aug 20, 2024

Description

One Line Summary

Add a null check on the permission list returned by onRequestPermissionResult, and handle it as a permission denial if the list is empty.

Details

Motivation

The current codebase assumes that onRequestPermissionResult always returns at least one permission. This can cause the app to crash in rare scenarios where the permission request is interrupted, resulting in an empty permission list. We have now added handling for this situation, and the permission will be denied by default if this occurs.

Scope

Both notification and location permission requests will be affected and will be handled the same in case the permission list returned from system is empty.

Testing

Manual testing

NOTE: I am unable to force reproduce the crash either manually or by simulation. However, I have tested other normal permission requesting behaviors to ensure that it does not interrupt the existing permission features.

Affected code checklist

  • Notification permission requesting
  • Location permission requesting

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

@jinliu9508 jinliu9508 added the WIP Work In Progress label Aug 20, 2024
@jinliu9508 jinliu9508 changed the title WIP: [Fix] Permissions returned by onRequestPermissionResult is empty [Fix] Permissions returned by onRequestPermissionResult is empty Aug 20, 2024
@jinliu9508 jinliu9508 removed the WIP Work In Progress label Sep 3, 2024
@jinliu9508 jinliu9508 merged commit 9df5484 into main Sep 5, 2024
2 of 3 checks passed
@jinliu9508 jinliu9508 deleted the fix-empty-permissions branch September 5, 2024 16:16
@jinliu9508 jinliu9508 mentioned this pull request Sep 5, 2024
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