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(share_plus): return correct share result on android #1301

Merged
merged 3 commits into from Oct 31, 2022

Conversation

Coronon
Copy link
Contributor

@Coronon Coronon commented Oct 28, 2022

Description

The share_plus android shareWithResult and shareFilesWithResult implementations are broken. The returned ShareResult.raw is always null on successful share. This PR fixes this by using FLAG_MUTABLE instead of FLAG_IMMUTABLE, as pointed out by @aakash-pamnani. This issue was not found when initially implementing the shareWithResult methods, as the FLAG_IMMUTABLE was only required by the highest API versions. Back then, a simple warning was emitted that strongly suggested using FLAG_IMMUTABLE, which seemed to break nothing.

This PR now fixes the behavior and ensures that the FLAG_MUTABLE PendingIntent given out may not be abused by moving from an implicit internal Intent to an explicit one. This also reduces mental complexity of the implementation.

I also added some more documentation throughout the android implementation.

As always, I provide share_plus_example as a pre-setup example project for this PR.

Related Issues

Fixes #916

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the pubspec.yaml files.
  • All existing and new tests are passing.
  • The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
  • No, this is not a breaking change.

Coronon added a commit to Coronon/share_plus_example that referenced this pull request Oct 28, 2022
@Coronon
Copy link
Contributor Author

Coronon commented Oct 28, 2022

The Linux test failed because it got a 404 while installing packages through apt - that should not affect this PR

Thank you for re-running the timedout Android integration tests :)

Copy link
Collaborator

@vbuberen vbuberen left a comment

Choose a reason for hiding this comment

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

Tested and it seems to be working good.

Left a few Kotlin related comments, but otherwise looks good to me.

@Coronon
Copy link
Contributor Author

Coronon commented Oct 29, 2022

Thanks for your review! Fixed all points mentioned and updated my example project and all references to it :)

PS: I just submitted a PR for the failing Linux test #1305

@miquelbeltran
Copy link
Member

LGTM, but I'll have to run this on Monday before merging.

@miquelbeltran miquelbeltran added Hacktoberfest Issues taking part in Hacktoberfest hacktoberfest-accepted labels Oct 29, 2022
@vbuberen
Copy link
Collaborator

LGTM, but I'll have to run this on Monday before merging.

I have tested it on Android 10 physical device and emulator of Android 12. Worked well.

@miquelbeltran miquelbeltran merged commit 994fef7 into fluttercommunity:main Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest Issues taking part in Hacktoberfest hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: SharewithResult always returning null in Android
3 participants