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 locationAlways does not work on Android 10 #921

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hirameee55
Copy link

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

This change fixes the regression described in Issue #917.

⤵️ What is the current behavior?

The current behavior is that Runtime Permission is not shown when requesting locationAlways on Android 10 (Q).

🆕 What is the new behavior (if this is a feature change)?

In Android Q (10), it is possible to request background and foreground permissions at the same time. LocationAlways satisfies this.
Android 11 (R) or higher cannot request background and foreground permissions at the same time, so v10.0.1 behavior is not affected.
These are Android specifications.
https://developer.android.com/training/location/permissions#request-background-location

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

When I request locationAlways on Android 10 (Q), the Runtime Permission appears and I can select "Allow all the time".

📝 Links to relevant issues/docs

🤔 Checklist before submitting

  • I made sure all projects build.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I followed the style guide lines (code style guide).
  • I updated the relevant documentation.
  • I rebased onto current master.

@hirameee55
Copy link
Author

@mvanbeusekom
I need the approval of the maintainer to have it reviewed.

Comment on lines +120 to +122

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R)
break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading through the documentation, I think we should always fall through to the location case and add the ACCESS_COARSE_LOCATION and/ or ACCESS_FINE_LOCATION as well. Meaning we need to remove the break; statement al together.

We should also improve the comment above to reflect this change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mvanbeusekom
Thank you for your review.

If remove the break;, #794 doesn't work.
Android R (11) and above cannot request background and foreground permissions at the same time.
Therefore, for Android R (11) and above, it is ideal to break if Manifest.permission.ACCESS_BACKGROUND_LOCATION is included and acquire permissions only in the background.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mvanbeusekom
Could you confirm my comments?
Please feel free to contact us if you have any questions.

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.

None yet

2 participants