Add support for Blob, ArrayBuffer, Uint8Array #28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that we support ArrayBuffer, we can do that very easily.
Bad copy and paste.
Bad news : I added a loop to apply a 0xFF mask. this will slow down a
bit the method.
Good news :
* JSZip.load should be more robust (sometimes the 0xFF wasn't
used, when using lastIndexOf for example).
* Now that we are sure that the string is in the right format, we can
use String.slice to read a string. The old method was :
foreach byte :
charCodeAt, & 0xFF, fromCharCode, append to the result string
This should compensate for the initial loop.
* Debugging is less painful : the old method triggered 1 call to
readByte... per byte. With "big" zip files (let say 10M), this would
lead to 10M method calls. The "with firebug activated" case was on my
tests 10 times slower than the "without firebug".
Now that we only do 1 method call (String.slice) to read to content
of a file, the debugger overhead has been reduced.
* JSZip.load(Uint8Array|ArrayBuffer) is faster : we avoid the useless
charCodeAt, & 0xFF, fromCharCode.
We now support * file(name, string) * file(name, Uint8Array) * file(name, ArrayBuffer) and * file(name).asText() * file(name).asBinary() * file(name).asUint8Array() * file(name).asArrayBuffer()
When a file is added with a content as a base64 string (with the base64 attribute), the asText() method didn't decode the text.
Tested browsers : IE 6/7/8/9 Firefox 3/3.6/4/10(esr)/17(esr) Chrome latest Safari 5.1/6 I updated jQuery because it seems to fix an issue with some zip files... Add JSZip.support because testing support might be hard. Example : typeof Blob !== "undefined" is not enough : Firefox 4 and Safari 5 know about Blob but not how to create them (no Blob constructor, no BlobBuilder). With a JSZip.support.blob, I can use it in the unit tests and the user can use generate() or asBlob() without any try/catch blocks.
|
Thanks David. I'll attempt to review the code by Wednesday. |
| */ | ||
| function StreamReader(stream) { | ||
| this.stream = stream; | ||
| this.stream = ""; | ||
| if (typeof Uint8Array !== "undefined" && stream instanceof Uint8Array) |
There was a problem hiding this comment.
Would it be better to use JSZip.support.uint8array? and JSZip.support.arraybuffer below?
|
Sorry, a bit late. All looks good, just a couple of comments :) |
|
Thanks for the review, I'll fix that :) I have a question about the coding style and brackets : should I prefer if (test)
{
doStuff();
}or if (test) {
doStuff();
}? I just found the commit 1459728 which changes some brackets from the former to the latter, but not everywhere. In this pull request, I used the first option but I can update all the files to use the good coding style. |
|
Thanks for the updates. I now prefer the latter as it's shorter without sacrificing readability. |
|
I'll update the code then :) |
See pull request 28 :
Change
if (test)
{
doStuff();
}
to
if (test) {
doStuff();
}
Also remove an extra (unused) `this` and fix a typo.
Stuk
added a commit
that referenced
this pull request
Jan 14, 2013
Add support for Blob, ArrayBuffer, Uint8Array
|
Great, thanks David! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These commits add :
generate({base64:true})forgenerate({type:"base64"})I tested it in :
This should resolve the issues #23 and #27