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

Add BinaryBlob Support for IE11 #1078

Closed
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@DrYSG
Copy link

DrYSG commented Dec 2, 2013

Added support for IE11 and IE12 blobs by using Uint8Array instead of string buffer for data, and then using a new faster MD5 crypto algorithm

daleharvey and others added some commits Dec 2, 2013

@@ -229,23 +229,38 @@ function IdbPouch(opts, callback) {
"Attachments need to be base64 encoded");
return PouchUtils.call(callback, err);
}
att.digest = 'md5-' + PouchUtils.Crypto.MD5(data);
var buffer = new Uint8Array(data);

This comment has been minimized.

@daleharvey

daleharvey Dec 2, 2013

Member

This crashes with Error: invalid arguments in firefox

This comment has been minimized.

@DrYSG

DrYSG Dec 3, 2013

I don't know what to tell you. While I stopped using FireFox a while ago (slow IDB blob performance) I just loaded the latest version and it did not error for me. (FireFox 25.0.1)

FireFox claims that it supports the constructor Uint8Array(string) support: see: https://developer.mozilla.org/en-US/docs/Web/API/Uint8Array

so there is no reason why it should be saying invalid arguments.

Try it with this line instead

var buffer = new Uint8Array(att.data);

But I think that what has to be store is base64, not a string.

This comment has been minimized.

@calvinmetcalf

calvinmetcalf Dec 3, 2013

Member

you're trying to create a typed array from a string firefox throws an error but chrome just silently ignores the string. You need to create the typed array from an actual array.

This comment has been minimized.

@DrYSG

DrYSG Dec 3, 2013

OK, I am still a bit baffled why it seemed to work for me with FireFox v25 (perhaps I don't stress it properly).

But lets say that I put in a check for the case of FireFox

data = (window.navigator.userAgent == "Mozilla")? split(data): data;
var buffer = new Uint8Array(data);

This can't be good PouchDB style for handing browser specific hacks, so what is the preferred method? Or do we want to pay the cost of the split for every browser?

This comment has been minimized.

@calvinmetcalf

calvinmetcalf Dec 3, 2013

Member

two issues

  1. all browsers claim to be Mozilla
  2. why exactly are you trying to create a typed array from a string?

This comment has been minimized.

@DrYSG

DrYSG Dec 3, 2013

what you say in (1) is true (but there are ways to pull things apart, but it can be fragile, look what IE12 claims it is now !!)

But (2) is more to the point. I am trying to get BinaryBlob support working again in IE11 and IE12, The issue with the old code was that IE does not support FileReader.readAsBinaryString (see https://developer.mozilla.org/en-US/docs/Web/API/FileReader.readAsBinaryString)

So I am using the new typed array reader which seems to have more cross browser support (and is the ONLY way in IE11 and IE12)

readAsArrayBuffer method (see
http://msdn.microsoft.com/en-us/library/ie/hh772312(v=vs.85).aspx )

[I am wide open to better ideas on how to do this, but what I found was this was working for me in for my binary blob storage in FF, IE and most important for me in Chrome]

This comment has been minimized.

@calvinmetcalf

calvinmetcalf Dec 3, 2013

Member

I think your going to have to go through and create an array base on the character codes at each position on the string as the best bet

This comment has been minimized.

@daleharvey

daleharvey Dec 3, 2013

Member

The tests are what showed this error, there is documentation on getting the test suite running @ https://github.com/daleharvey/pouchdb/blob/master/CONTRIBUTING.md#running-pouchdb-tests

in particular its the http://127.0.0.1:8000/tests/test.html?testFiles=test.attachments.js that fails (all of them)

DrYSG added some commits Dec 2, 2013

Merge branch 'IE11' of https://github.com/DrYSG/pouchdb into IE11
Added conversion from string to Uint8Array for string attachments
Merge branch 'IE11' of https://github.com/DrYSG/pouchdb into IE11
Added conversion from string to Uint8 for string attachments
@DrYSG

This comment has been minimized.

Copy link

DrYSG commented Dec 4, 2013

I updated the source to do a manual conversion from string to typed array (Uint8Array). This should remove the run-time error.

But this work should really be reviewed by someone who either authored the IDB interface, or the current stake-holder.

Why? I admit my complete ignorance in knowing why there is a special branch for string attachments, and why this is converted to Base64. Perhaps itcould just convert to a typed array and then store as binary blob? Basically, I am saying that the branch I need (binary blob) works just great, but someone who really understands this should look at the global implications.

@daleharvey

This comment has been minimized.

Copy link
Member

daleharvey commented Dec 5, 2013

I will take an extra look at this tomorrow, the api is exposed for people to interchangeably read and write with either blobs or base64, it gets confusing since different backend support different capabilities

Any change on this level will likely hit against something in the tests so if you can get them to run it would really help you, there is docs up @ https://github.com/daleharvey/pouchdb/blob/master/CONTRIBUTING.md and if you have any questions just ask

@DrYSG

This comment has been minimized.

Copy link

DrYSG commented Dec 5, 2013

I agree about the tests, it is just a combination of out firewall and the fact that I have to sit down and learn about node and node packages in depth. I have blocked out a couple of weeks later this month, but I am not there yet, and my initial
"assume it is all easy and intuitive" attempts have failed.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Feb 3, 2014

Revisiting this, I can confirm that in IE11 the attachments tests do not pass. Annoyingly there's no error message; it just hangs.

In case it facilitates the development process, there's now pouchtest.com, so you don't have to run the dev server yourself. http://pouchtest.com/tests/test.html?testFiles=test.attachments.js will run the attachments test.

@daleharvey

This comment has been minimized.

Copy link
Member

daleharvey commented Feb 18, 2014

Closing due to inactivity, please reopen if needed, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment