cordova-plugin-file icon indicating copy to clipboard operation
cordova-plugin-file copied to clipboard

fix(android): Content FS support in PathHandler

Open breautek opened this issue 1 year ago • 0 comments
trafficstars

Platforms affected

Android

Motivation and Context

Improves support for the webview asset loader Path handling for easier DOM access, especially for content:// urls.

Description

There are 3 main changes:

  1. Assets filesystem check is unsafe

targetFileSystem == "assets";

does not do what might have been expected, the condition will always yield false. This is because in Java, comparing an object to a string literal using a == operator is a reference equality check. That is it's not testing if the values are equal, it's testing if they are the same object. Because a string literal is always making a new object to evaluate this line, it will always be false.

This was corrected to use .equals which does a value equality check.

  1. Support content filesystem.

The existing path handler supported many different filesystems, but content was missing. Adding support for the content filesystem required a few other changes.

Including:

  • Adding the mapping
  • Parsing path segments (which is a decoded form) and re-encoding them for content paths. This is important because the content path must be rebuilt exactly how the Android OS originally produced it, otherwise it will produce permission issues.
  • Replacing raw file API usage with CordovaResourceApi which intelligently checks for the uri type and calls the appropriate API to handle reading the resource.

Testing

Ran npm test.

~Paramedic tests isn't running locally and I'm not sure why, so we'll see if it runs on the CI.~

I also have a test project that I used to test this change specifically:

cameraTest.zip

Note: The camera plugin was tested with https://github.com/apache/cordova-plugin-camera/pull/889 PR.

Note: The failing iOS change is not a regression of this PR, as can be observed it was failing on the 8.1 release. Not sure when that test started failing.

Checklist

  • [x] 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)
  • [x] I've updated the documentation if necessary

breautek avatar Jul 18 '24 19:07 breautek