search

ebidel / idb.filesystem.js

HTML5 Filesystem API polyfill using IndexedDB
https://www.npmjs.com/package/idb.filesystem.js
Other
488 stars 46 forks source link

Fix IE10 #13

Closed Sebmaster closed 11 years ago

Sebmaster commented 11 years ago

As reported in #3 I'll try to fix idb.filesystem.js for IE10.

This first commit only removes the uses of defineSetter and defineGetter, I'm still getting a INVALID_MODIFICATION_ERR which I've yet to debug, but I wanted input on the following:

Is there a reason you need the getter/setter anyways? Why not use the prototype? This is the only use of a setter where it's (under my impression) a sensible thing to do; are there problems if the properties are not defined with setters?

Sebmaster commented 11 years ago

This fixes IE (mostly). The tests still use __proto__ which seems to be unsupported in IE; these tests fail, the others pass.

I'm sure there's still something wrong in regards to file retrieval since IE still fails with my project but I can't look into that right now. If noone else takes over I'll probably get to it in a week or so.

ebidel commented 11 years ago

Thanks for this! Getting to it now that Google IO is over :)

Love moving to ES5 getters. Imchose to use getters mainly so those properties are truly read only. Other than that, there's no reason.

Sebmaster commented 11 years ago

Oh god, I totally forgot about this pull request too. I'm not sure anymore what the problem was with IE, not sure if there even was one or if I just fucked up somewhere and corrected it later. I'll take a look at it shortly.

Sebmaster commented 11 years ago

It looks like files aren't saved in the file system. The entries are in there, but the assigned blob isn't. I'm not sure yet if that's because of an application error, the "file" type or because IE can't save blobs in IndexedDB at all.

The passing tests make me believe it's an application error (compatible with chrome though), it would be nice if someone can verify that though.

erdbeerschnitzel commented 11 years ago

I'm not sure how to read this discussion but in my tests the changes by Sebmaster didn't help. With these changes applied it even doesn't work anymore in Firefox.

If anyone just wants to make it work, take Sebmasters last commit and remove the getters/setters for file (lines 380 - 387) and blob (167 - 177). I know the getters/setters are there for a reason :)

Sebmaster commented 11 years ago

The file_ setter is unnecessary.

I just tested FF and IE10 again and got them to work in osu-web. The tests also all work. Are you sure? Code examples maybe?

The mime type seems to be absolutely necessary in order to get (FF and) IE to work. IE also saves the file_ correctly now.

Sebmaster commented 11 years ago

Okay @erdbeerschnitzel is correct. That's definitely something the tests didn't catch. Setting enumerable to true in the defineProperty objects fixes that though.

Sebmaster commented 11 years ago

Rebased onto master too. Should be mergable now.