WICG / entries-api

Spec defining browser support for file/directory upload by drag-and-drop
https://wicg.github.io/entries-api/
Other
41 stars 9 forks source link

Specify more types/members? #4

Closed inexorabletash closed 7 years ago

inexorabletash commented 8 years ago

Mozilla sources:

... would seem to indicate that the following are being implemented:

Per comment in Moz's source, other methods on FileSystemEntry (moveTo(), copyTo(), etc) are not being implemented.

@aliams - has Edge seen the need to implement any of the above for compat?

aliams commented 8 years ago

We have the filesystem attribute on Entry as well as the name on DOMFileSystem. All others haven't been implemented since we haven't seen those APIs being used in the folder upload case.

inexorabletash commented 8 years ago

Thanks @aliams ! Does filesystem return a DOMFileSystem instance w/ name and root? And are you using anything particular for name (based on compat) or should I spec that as implementation dependent? (Chrome uses something like "http_localhost_8888:Isolated_5DF560229F346CB962E051AE5FBC57D1")

aliams commented 8 years ago

Yes, it returns a DOMFileSystem instance that has both name and root. The name we use is just as random, mostly because its value shouldn't matter as far as the developer is concerned. It's possible that it isn't actually needed for the folder upload case.

inexorabletash commented 8 years ago

Okay. I'll go ahead and specify - thanks!

smaug---- commented 8 years ago

Gecko has also DOMFileSystem with name and root. name is randomly generated.

inexorabletash commented 8 years ago

Went ahead and specified FileSystemFlags since the binding behavior is observable and Gecko implemented the special case for create.

Change is: 89ccc1fde7e907bf967acf05bff1ed276583833a

inexorabletash commented 7 years ago

@bakulf - do you know why Gecko retains FileSystemDirectoryEntry.removeRecursively() and FileSystemFileEntry.createWriter() as methods that only throw? Were they needed for compat?

bakulf commented 7 years ago

Before this Entries API spec, I wanted to expose exactly the same Blink FileSystem API as Chrome does, but implementing only the 'read-only' part of it. I also wrote patches to implement exactly what the Entries API says, but I would like to ship them together with Chrome for compatibility reasons.

inexorabletash commented 7 years ago

@bakulf — that raises two questions:

bakulf commented 7 years ago

Question 1: I don't think we should have those methods in this spec. The reason why I wanted to include them, is because, if a website does: entry.removeRecursively(success), FF doesn't throw. Same thing for createWriter(). Now, I've never seen this thing really happening when testing the main websites such as dropbox, google drive etc.

Question 2: those patches are now out of sync. I have to update them. At that time we didn't have FileSystem in in FileSystemEntry, the callbacks were implemented differently and so on. I think the current state of the spec is good. I'll update them to the latest spec soon.

Do you know when Chrome code will be updated?

smaug---- commented 7 years ago

What affected to Gecko implementation was the plan the Edge had at the time. They shared the interfaces they were going to implement and we tried to match that.

aliams commented 7 years ago

For what it's worth, I would recommend leaving those methods out of the spec since they do not directly apply to the directory upload scenarios.

bakulf commented 7 years ago

I agree. We just removed removeRecursively in gecko. I'm going to remove createWriter() as well.

inexorabletash commented 7 years ago

Excellent. I'll resolve this then.

As far as when Chrome will be updated - I ran into internal dependencies (i.e. web-facing) with trying to change the names - we use the same types for the filesystem API exposed to plugins/extensions/etc - and some of those actually tested the names via Object.prototype.toString.call(o); I bumped it down on my priority list but will get back to it this month.