Skip to content

fix(windows): readAsArrayBuffer to use openSequentialReadAsync #298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

knight9999
Copy link
Contributor

Platforms affected

cordova-windows

Motivation and Context

Before this PR, readAsArrayBuffer does not work on windows 10.
This is because xhr fails with empty error.
Indeed some specs in tests fail as
2019-03-06 11 17 22

This PR is to fix this issue.

Description

Instead of using MSApp.createFileFromStorageFile, URL.createObjectURL and XMLHttpRequest, I used Windows.Storage.Streams.DataReader which is a standard method for reading binary files.

Testing

With the help of cordova-mobile-spec, I confirmed all tests pass in cordova-plugin-file-tests.tests on Windows 10 PRO 1809.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • 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)
  • I've updated the documentation if necessary

@knight9999 knight9999 requested a review from erisu March 6, 2019 02:56
@janpio janpio changed the title fix readAsArrayBuffer to use openSequentialReadAsync fix(windows): readAsArrayBuffer to use openSequentialReadAsync Jul 4, 2019
@janpio janpio closed this Jul 4, 2019
@janpio
Copy link
Member

janpio commented Jul 4, 2019

Restart the tests.

@janpio janpio reopened this Jul 4, 2019
@janpio janpio closed this Jul 4, 2019
@janpio
Copy link
Member

janpio commented Jul 4, 2019

CI: Once more please.

@janpio janpio reopened this Jul 4, 2019
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.

3 participants