You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by GitBox <gi...@apache.org> on 2021/09/08 23:52:55 UTC
[GitHub] [cordova-plugin-file-transfer] kpatfln opened a new pull request #310: [Android] Enable using MediaStore API on Android Q
kpatfln opened a new pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
Android
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
On Android Q and above, the download functionality does not function.
### Description
<!-- Describe your changes in detail -->
This change enables use of MediaStore to save files into the download folder for Android Q and above, while still retaining the existing logic for older versions of Android.
### Testing
<!-- Please describe in detail how you tested your changes. -->
Without the changes, Android Q will fail to download, while on Android P and below it will work fine.
With the changes, Android Q and above will have no problems with downloads.
### Issues Resolved
https://github.com/apache/cordova-plugin-file-transfer/issues/300
https://github.com/apache/cordova-plugin-file-transfer/issues/291
### Checklist
- [ ] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
- [X] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [X] 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](https://help.github.com/articles/closing-issues-using-keywords/))
- [X] I've updated the documentation if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-plugin-file-transfer] kpatfln commented on a change in pull request #310: [Android] Enable using MediaStore API on Android Q
Posted by GitBox <gi...@apache.org>.
kpatfln commented on a change in pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310#discussion_r707893214
##########
File path: src/android/FileTransfer.java
##########
@@ -792,7 +796,24 @@ public void run() {
// write bytes to file
byte[] buffer = new byte[MAX_BUFFER_SIZE];
int bytesRead = 0;
- outputStream = resourceApi.openOutputStream(targetUri);
+ /*
+ This ensures that for Android Q and above that the new MediaStore API being used.
+
+ Instead of using the provided target directory for saving from the web app, it will automatically
+ save to the Downloads folder.
+ */
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
Review comment:
Thanks for your advice!
I agree that the limitation of just throwing it into the Downloads folder mightn't be the best idea.
Let me give it a try with `cordova-plugin-file` and get back to you with the PR there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-plugin-file-transfer] kpatfln commented on a change in pull request #310: [Android] Enable using MediaStore API on Android Q
Posted by GitBox <gi...@apache.org>.
kpatfln commented on a change in pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310#discussion_r707893214
##########
File path: src/android/FileTransfer.java
##########
@@ -792,7 +796,24 @@ public void run() {
// write bytes to file
byte[] buffer = new byte[MAX_BUFFER_SIZE];
int bytesRead = 0;
- outputStream = resourceApi.openOutputStream(targetUri);
+ /*
+ This ensures that for Android Q and above that the new MediaStore API being used.
+
+ Instead of using the provided target directory for saving from the web app, it will automatically
+ save to the Downloads folder.
+ */
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
Review comment:
Thanks for your advice!
I agree that the limitation of just throwing it into the Downloads folder mightn't be the best idea.
~~Let me give it a try with `cordova-plugin-file` and get back to you with the PR there.~~
Done! I've edited the commit with using `FileOutputStream` instead and it works brilliantly on API 29 and API 30.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-plugin-file-transfer] breautek commented on a change in pull request #310: [Android] Enable using MediaStore API on Android Q
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310#discussion_r705368872
##########
File path: src/android/FileTransfer.java
##########
@@ -792,7 +796,24 @@ public void run() {
// write bytes to file
byte[] buffer = new byte[MAX_BUFFER_SIZE];
int bytesRead = 0;
- outputStream = resourceApi.openOutputStream(targetUri);
+ /*
+ This ensures that for Android Q and above that the new MediaStore API being used.
+
+ Instead of using the provided target directory for saving from the web app, it will automatically
+ save to the Downloads folder.
+ */
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
Review comment:
Thank you for your effort in preparing this PR.
I just wanted to point out that this doesn't really honour the existing [download](https://github.com/apache/cordova-plugin-file-transfer#download) API, as it ignores the target download path and just throws the downloaded file in the Downloads container as also stated in your code comment. I think is a bad assumption, and I believe this is an unnecessary limitation. I'll explain further.
We can still use file paths. The difference in API 29+/Scoped Storage is not all external file paths are writable (e.g. the external root directory is no longer a writable path).
We don't need to use the `MediaStore` APIs directly, we can use MediaStore via using [Direct File Paths](https://developer.android.com/training/data-storage/shared/media#direct-file-paths). While MediaStore APIs is the recommended approach by Google, but it only really works if you can make these kind of assumptions. From a Cordova standpoint, I don't think we can make these kind of assumptions. I've done some testing with this already against the `cordova-plugin-file` and it appears to work.
Interested to hear your thoughts on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-plugin-file-transfer] kpatfln commented on pull request #310: [Android] Enable Download Functionality with Android Q
Posted by GitBox <gi...@apache.org>.
kpatfln commented on pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310#issuecomment-926251660
@erisu Hi - I've completed the suggested changes. How does it look? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-plugin-file-transfer] kpatfln commented on a change in pull request #310: [Android] Enable using MediaStore API on Android Q
Posted by GitBox <gi...@apache.org>.
kpatfln commented on a change in pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310#discussion_r707893214
##########
File path: src/android/FileTransfer.java
##########
@@ -792,7 +796,24 @@ public void run() {
// write bytes to file
byte[] buffer = new byte[MAX_BUFFER_SIZE];
int bytesRead = 0;
- outputStream = resourceApi.openOutputStream(targetUri);
+ /*
+ This ensures that for Android Q and above that the new MediaStore API being used.
+
+ Instead of using the provided target directory for saving from the web app, it will automatically
+ save to the Downloads folder.
+ */
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
Review comment:
Thanks for your advice!
I agree that the limitation of just throwing it into the Downloads folder mightn't be the best idea.
---Let me give it a try with `cordova-plugin-file` and get back to you with the PR there.---
Done! I've edited the commit with using `FileOutputStream` instead and it works brilliantly on API 29 and API 30.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-plugin-file-transfer] erisu merged pull request #310: fix(android): enable download functionality with Android Q
Posted by GitBox <gi...@apache.org>.
erisu merged pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-plugin-file-transfer] kpatfln commented on pull request #310: [Android] Enable using MediaStore API on Android Q
Posted by GitBox <gi...@apache.org>.
kpatfln commented on pull request #310:
URL: https://github.com/apache/cordova-plugin-file-transfer/pull/310#issuecomment-915712635
@erisu Removed the whitelist code - it wasn't necessary. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org