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

IE11 and binary blobs #900

Closed
DrYSG opened this Issue Aug 8, 2013 · 12 comments

Comments

Projects
None yet
6 participants
@DrYSG
Copy link

DrYSG commented Aug 8, 2013

I have some time to start looking into IE11 support for putting attachments for binary blobs. What direction would be most fruitful for the team for me to explore (e.g. adding a script to compute MD5SUM?).

BTW, I have a demo I can throw up on CodePen.

Here is the error I get when I try to put a binary blob attachment:

// pouchdb.nightly - 2013-07-31T02:01:34

SCRIPT438: Object doesn't support property or method 'readAsBinaryString'File: pouchdb-nightly.min.js, Line: 3, Column: 12826


Want to back this issue? Place a bounty on it! We accept bounties via Bountysource.

@DrYSG

This comment has been minimized.

Copy link

DrYSG commented Aug 9, 2013

I have created the demo on CodePen. Try: http://codepen.io/DrYSG/pen/kdzft with Chrome, FireFox, and IE10 or IE11. Chrome will work. FireFox is getting issue #901.

Just press [Download], it will clear any old DB. If you want to manually remove the PouchDB there is a [Delete DB] button to give you a clean slate.

One thing I had to add to this demo is a variable called maxThreads, which is currently set to 50 for Chrome, and 1 for Firefox. You can change the values. Chrome has an issue with SPDY protocols, it allows too many asynch XHR calls, and it kills the browser (runs out of memory and thread pool). So there some fancy code using jquery promises to make sure that there is a maximum of outstanding XHR2 calls.

@DrYSG

This comment has been minimized.

Copy link

DrYSG commented Aug 19, 2013

Well, I have gone into the source, and created a fix that works for me in IE11, FF24, and Chrome V28. To run this demo:

  1. Run http://codepen.io/DrYSG/pen/kdzft
  2. Press [Download]

It will load 171 JPG map tiles via XHR2 and stored them in PouchDB (The table tells you if Binary or Base64 (Chrome) is being used in PouchDB).

[If you want to try with more JPG and get a higher resolution map (600+ tiles) then change line 1 to
var serverURL = "https://googledrive.com/host/0B2Ay-nw1QSW2SHhjR0VBZXE2bUU/LittleBlueMarble/" ]

You can also [DELETE] the PouchDB databases.

Please tell me what performance you see. Behind my firewalls I am getting about 24 Files/Sec. in IE11 and Chrome
Also tell me if IE10 is working for you.
FireFox has a real poor IDB (an overlay of WebSQL, and no SPDY support, so performance is very very poor)

You can then look at the full downloaded map by going to:

http://codepen.io/DrYSG/pen/mcdCq


What did I change? Well firstly, I am unable to get the source to build, so I had to hack the minified nightly downloads (can someone help me with issue https://github.com/daleharvey/pouchdb/issues/902)

What I had to do is change the IDB adapter function preprocessAttachment() it now no longer does a readAsBinaryString() but does a readAsArrayBuffer() which returns a fancy HTML5 ArrayBuffer, which I convert to a Uint8Array()

I also upgraded the MD5 algorithm, using the excellent and faster version from Joseph Myers http://www.myersdaily.org/joseph/javascript/md5-text.html which is talked about in StackOverflow as the fastest. http://stackoverflow.com/questions/1655769/fastest-md5-implementation-in-javascript I saw about a 4 file/sec improvement in performance.

I am no expert in the internals of PouchDB, so I suspect I may have broken some things for some people. Also GitHub does not understand passworded proxies, so I have no way to do Pull requests to submit my changes.
But feel free to look at what I changed in PouchDB at https://drive.google.com/?usp=chrome_app#folders/0B2Ay-nw1QSW2b0dtcUM3ekdfWVE

This was referenced Aug 19, 2013

@daleharvey

This comment has been minimized.

Copy link
Member

daleharvey commented Aug 20, 2013

Going to take a look at your #902 issue now, but this work is great, apologies for the radio silence, its been a busy few weeks but will start getting up to speed and try to get this merged asap

As far as I know chrome does support either arraybuffers or blobs in idb despite that still being an open issue, I keep hearing conflicting reports so will test it all out

@DrYSG

This comment has been minimized.

Copy link

DrYSG commented Aug 20, 2013

Thank you,

I was about to write you. I can tell you that Chrome does support ArrayBuffers, but does not yet support blobs: https://code.google.com/p/chromium/issues/detail?id=108012 . So what I do is copy the data out of the ArrayBuffer into a string for Chrome. A small performance hit. But then the code was previously doing an btoa().

Thank you for looking at this. I was going to write you to ask what the likelyhood of something along these lines gets into the baseline distribution. I am seeing IE11 as giving Chrome a real run in terms of SPDY, JavaScript, and HTML5 performance. (FF is sadly lacking in these areas).

I realize that my lack of GitHub access, source copy, and PouchDB internals naivety, means that what I did is a bit of hack. But it does work for me, and hopefully it gives some help in getting IE support.

@DrYSG

This comment has been minimized.

Copy link

DrYSG commented Oct 8, 2013

Is there any plans to include this fix for storing Binary Blobs for IE (10 and 11) in the next version of PouchDB?

@DrYSG

@old9

This comment has been minimized.

Copy link

old9 commented Mar 14, 2014

This helps me on developing a Metro app, I noticed that you have forked the project, why not make a PR?

@DrYSG

This comment has been minimized.

Copy link

DrYSG commented Mar 14, 2014

I did have it as a pull request a few months ago. And the forked version still works. But I don't have the funding right now to do all the unit testing and integration work needed to get it into the baseline.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Mar 14, 2014

Sorry for the lack of responses. We would definitely look into a pull request if someone submitted one, but it's difficult to use the files you uploaded, since Pouch has changed considerably since then, and there's no diff.

Mostly likely though, the only code you'd need to modify is blob.js, and then the tests in test.attachments.js should take care of the rest.

I admit our IE support for the tests is a little lacking, though; last I checked, you can only run one test at a time, because there's some silent error in the test cleanup method. It's totally doable, though, and I've gone through the painstaking effort in a VirtualBox environment, one test at a time, as recently as here, so I know it's doable.

As an aside, anybody who's interesting in fixing all the things in IE would be a very welcome contributor to this project!

@trokster

This comment has been minimized.

Copy link
Contributor

trokster commented Mar 15, 2014

Hi,
Tested using readAsBufferArray instead of readAsBinaryString, then getting the string representation via this snippet I found on stack/flow, and it works with very little change to the codebase.

        var buffer = this.result;
        var binary = "";
        var bytes = new Uint8Array(buffer);
        var length = bytes.byteLength;
        for (var i = 0; i < length; i++) {
          binary += String.fromCharCode(bytes[i]);
        }

then we replace this.result with binary

Not very optimized, but very little impact to the codebase.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Mar 16, 2014

This seems like it may be fairly open and shut. Gonna tag goodfirstpatch.

@trokster

This comment has been minimized.

Copy link
Contributor

trokster commented Mar 17, 2014

Woo hoo, no tests needed normally :p

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Jun 5, 2014

IE10 is passing in Travis. Closing this.

@nolanlawson nolanlawson closed this Jun 5, 2014

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