<!--
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, iOS, Windows.

My organization hit this one in production in a highly visible (to the end 
user) setting. So I've gone ahead and fixed the 3 platforms we target. I 
haven't yet tested on OSX or browser, but looking at the source it looks like 
fixes are likely needed on those platforms too. I don't think that needs to 
hold up this PR, b/c I've made my change in a backwards compatible way. But 
that could be an area to discuss.

### What does this PR do?
Fixes [CB-13570](https://issues.apache.org/jira/browse/CB-13570) on the 
specified platforms. Specifically, this PR changes the JS-to-native interface 
for the readAsX methods. Previously the JS side expected the native side to 
return the read value (be it a text string, ArrayBuffer, data URL, etc.) With 
this PR, the JS side now can handle two result formats from the native side:
1. An object like `{ value: any, numBytesConsumed: number }`, where `value` is 
the read value (text, ArrayBuffer, etc.) and `numBytesConsumed` is the number 
of bytes consumed to read that value. `numBytesConsumed` can differ from the 
specified `READ_CHUNK_SIZE` for reasons that will be clear shortly.
1. The previous format, for backwards compatibility with platforms/read methods 
that haven't yet had their native sides updated for this change.

Then, on Android, iOS, and Windows, the native side uses this new flexibility 
to change its handling for readAsText specifically. Depending on the specified 
encoding, if the end offset requested by the JS side would cause a multi-byte 
character to get split, the native side extends the end offset as needed to 
prevent splitting. The native side then returns an accurate `numBytesConsumed` 
to reflect the extra bytes needed.


### What testing has been done on this change?
I added an automated test that exposes the bug and now passes on the fixed 
platforms. I also did manual testing in the app that exposed the bug for us.

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 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.
- [x] Added automated test coverage as appropriate for this change.


[ Full content available at: 
https://github.com/apache/cordova-plugin-file/pull/242 ]
This message was relayed via gitbox.apache.org for [email protected]

Reply via email to