CB-11534 This plugin should support Android Marshmallow runtime permission

[homen]

Platforms affected

Android

What does this PR do?

It will support Android Marshmallow runtime permission.

What testing has been done on this change?

Tested in Android.

Checklist

• [x] ICLA has been signed and submitted to secretary@apache.org.
• [x] Reported an issue in the JIRA database
• [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
• [ ] Added automated test coverage as appropriate for this change.

[cordova-qa]

Cordova CI Build has one or more failures.

Commit - Link Dashboard - Link

Builder Name	Console Output	Test Report	Device Logs
Windows 8.1 Store	Link	Link	Link
Windows 10 Store	Link	Link	Link
Windows 8.1 Phone	Link	Link	Link
iOS	Link	Link	Link
Android	Link	Link	Link

[homen]

What is the error?

[vladimir-kotikov]

@homen, there is nothing wrong with this PR (at least from CI perspective) - the failure was caused by some test defects on iOS platform. We're aware of that issue and investigating it.

[homen]

Ok @vladimir-kotikov

[huhaixiang]

When to support the Android 6 platform? Thanks

[homen]

@huhaixiang It is about Runtime Permission.You can check here link

[infil00p]

Hey. This should really add the cordova-compat plugin if you're going to start adding permissions to plugins.

[homen]

@infil00p I do'nt understand?What I need to do?

[naveedahmed1]

Can this be merged? So that it can be used on Android 6 or higher.

[maxpaj]

Any progress on this?

I can confirm that this works on Android 6.0. Before, my app couldn't access any files. With homens changes I got the permission popup and then my app could access the files.

[luckylooke]

Same as @maxpaj I can confirm this is a fix for permission issues on Android 6+, is there any reason this is not merged yet??

[maxpaj]

This should really be merged...

[ygrishajev]

@homen you should probably import cordova.PermissionHelper like here and set a dependency for cordova-plugin-compat like here

[stalniy]

@homen do you plan to fix the PR in the nearest future? :) It's really annoying for me... And I want to have this fix in my codebase!

[homen]

Hi all, I have done the required changes. It can be merged now.

[cordova-qa]

Cordova CI Build has one or more failures.

Commit - Link Dashboard - Link

306 tests run, 15 skipped, 148 failed.

[homen]

@filmaj All my editors showing correct alignment. Anyhow I fixed this with five commits !

[cordova-qa]

Cordova CI Build has one or more failures.

Commit - Link Dashboard - Link

306 tests run, 15 skipped, 149 failed.

[cordova-qa]

Cordova CI Build has one or more failures.

Commit - Link Dashboard - Link

306 tests run, 15 skipped, 148 failed.

[cordova-qa]

Cordova CI Build has one or more failures.

Commit - Link Dashboard - Link

306 tests run, 15 skipped, 153 failed.

[filmaj]

Thanks. I will need to test this manually as the file-transfer CI is super broken.

[stalniy]

@filmaj any news on this?

[filmaj]

Alright, I took a look at this again after an extended break and I have more comments, and want to bring more information to the community about the future of this plugin as well.

First, and I think most important, we need to be clear about the future of this plugin. A couple of months back, Cordova developers did an audit of the core cordova plugins, and one result was the decision to sunset the File Transfer plugin in favour of the increasingly-available XHR2 APIs. The XHR2 (now simply merged into the core XHR spec) features that are relevant to File Transfer plugin users are the ability to upload and to receive progress events - giving us exact feature parity for what File Transfer provides to users today. The benefit to sunsetting this plugin is that there would be less stuff to maintain for Cordova as XHR2 is already widely available on most mobile platform. With that in mind, that makes me reluctant to make many changes to this plugin as it's essentially makework.

Second, looking at the specific code changes in here, I started wondering, why execute the permission requesting inside this plugin? This plugin already depends on the core cordova File plugin, which does have support for Marshmallow permission model - why duplicate that? Turns out it is because the read/write methods of File Transfer, on the native Android side, are completely recreated/reproduced! I think a "proper" change to FT to support this would be to leverage cordova-plugin-file's FileUtils class and call its execute method, passing in the right arguments to trigger the write action. Similarly, for reading files, the same could be done, using one of the plethora of readAs* methods available in cordova-plugin-file's FileUtils.java class. The benefits I see to using this approach:

1. no need to rewrite permission handling code - cordova-plugin-file's native Android code already does that.
2. we could completely remove all file-reading and file-writing native Android code inside cordova-plugin-file-transfer, as we'd defer all of that work to the File plugin.

All that being said, once Cordova sunsets this plugin, the plugin code will be open and available to the community. The community could manage this code as it is however it seems right to them. While the patch here is not "perfect", it works, and that is more valuable to a lot of people.

So, I think I won't take any action on this PR. I am sorry that my previous review and comments in here made it seem like this PR would make its way into the mainline. Things have changed, and this plugin's window of life is closing.

[maverickmishra]

With the new features introduced in XMLHttpRequest, this plugin is not needed any more. Migrating from this plugin to using the new features of XMLHttpRequest, is explained in this Cordova blog post.