Skip to content

Download external media on Share intent received#10824

Merged
malinajirka merged 2 commits intodevelopfrom
fix/save_remote_uris_in_share_receiver
Nov 27, 2019
Merged

Download external media on Share intent received#10824
malinajirka merged 2 commits intodevelopfrom
fix/save_remote_uris_in_share_receiver

Conversation

@planarvoid
Copy link
Copy Markdown
Contributor

@planarvoid planarvoid commented Nov 21, 2019

Fixes #10575
This crash is probably triggered because the content system URIs are only available for a short time period after they are shared with our app. When the user has multiple sites we show a site selector first before we pass the URIs to the MediaBrowserActivity which downloads them. This PR changes this behaviour and saves the files directly when they are shared with the app. This should fix the crash.

I've also found out there is a crash when you try to share a video with the app (tried on an emulator with the latest android). I think it's related to react-native-image-picker/react-native-image-picker#1139

To test:

  • Try to share an image (from Google Photos) with the app
  • On the site selector rotate the screen
  • Select "Add to media library"
  • Notice that the files are uploaded in the media library

To test 2:

  • Try to share an image (from Google Photos) with the app
  • Select "Add to new post"
  • Notice that a post is created with the shared image

To test 3:

  • Try both scenarios with multiple images

PR submission checklist:

  • I have considered adding unit tests where possible.

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@planarvoid planarvoid added this to the 13.8 milestone Nov 21, 2019
@planarvoid planarvoid self-assigned this Nov 21, 2019
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 21, 2019

You can test the changes on this Pull Request by downloading the APK here.

Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @planarvoid! LGTM - the only thing we might want to consider is adding a comment that MediaUtils.downloadExternalMedia must be called on the UI thread (see CopyMediaToAppStorageUseCase). Wdyt?

@malinajirka
Copy link
Copy Markdown
Contributor

Btw I also noticed, that when you share a media and you are not logged in, the sign up flow is shown -> I believe that this scenario is not covered. However, it seems like a low priority issue.

@planarvoid
Copy link
Copy Markdown
Contributor Author

@malinajirka thanks for the check. I've added the comment to the downloadExternalMedia method in 7339e41

Btw I also noticed, that when you share a media and you are not logged in, the sign up flow is shown -> I believe that this scenario is not covered. However, it seems like a low priority issue.

I think this is unrelated to this issue and is indeed low priority

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RuntimeException: Unable to start activity ComponentInfo{org.wordpress.android/org.wordpress.android.ui.media.Media...

2 participants