Skip to content
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

entry.toURL() should create a blob url instead of filesystem URL #5

Open
ebidel opened this issue Apr 30, 2012 · 8 comments
Open

entry.toURL() should create a blob url instead of filesystem URL #5

ebidel opened this issue Apr 30, 2012 · 8 comments

Comments

@ebidel
Copy link
Owner

ebidel commented Apr 30, 2012

Unsupported browsers don't understand filesystem: URLs so they cannot be used out of the box.

The motivation for even implementing toURL() was to make the lib compatible with filer.js.

I should look into this.

@BenjaminDobler
Copy link

Hey any ideas how one could implement this?
The problem is see is that for filesystem toURL() is synchronous but creating a blob url can only be done asynchronous i think. One idea i have is to generate the blob urls when the blobs are stored and save them in a map so that one can access them later in a synchronous way.

@ebidel
Copy link
Owner Author

ebidel commented May 4, 2013

Right now .toURL() does what the Filesytem API does, which is to generate a filesystem: URL string.

Creating a blob and a blob: URL from is are also synchronous operations. There's a rerfernce
to the file stored is stored deep in the library, so you could do something like this:

function toURL(entry) {
  // TODO: cleanup URLs created using revokeObjectUR().
  if (entry.isFile && entry.file_.blob_) {
    var blob = entry.file_.blob_;
  } else {
    var blob = new Blob([]);
  }
  return window.URL.createObjectURL(blob);
}

That URL can then be used for previews and whatnot. I've updated the demo to do this.

Down the road, it might be nice to bake this in under the hood. I'd have to change some stuff around though
because the filer.js library relies on .toURL() internally.

Hopefully that helps.

@BenjaminDobler
Copy link

Ah yeah awesome. I got confused with another issue i`m facing. I really like about the filesystem that you can predict the file urls without actually creating a file. With blobs you first have to create the blob and then the url which is a synchronous act. So the idea was to create all the existing blobs from the db and create all the urls in advance but this might be a memory nightmare :-)

@ebidel
Copy link
Owner Author

ebidel commented May 8, 2013

It's an interesting idea, but we're talking possibly hundres of object URLs. I think you're right it's not the memory tradeoff.

@mudcube
Copy link
Contributor

mudcube commented Oct 13, 2014

I ended up replacing toURL with:

toURL: function() {
  return URL.createObjectURL(this.file_.blob_);
}

I think it makes more sense to make the URL actually work, otherwise is a bit confusing. The memory issue is interesting, but feel the tradeoff for usability is worth it. Perhaps a warning on the index would make sense, to remind programmers since it's a shim, there are some memory drawbacks to toURL and to handle accordingly.

@ebidel
Copy link
Owner Author

ebidel commented Oct 13, 2014

That might be worth doing, although revokeObjectURL should be called if something tries calling toURL() on the same blob. That would require some bookkeeping.

@mudcube
Copy link
Contributor

mudcube commented Oct 13, 2014

How about something like this?

toURL: function() {
    var blob = this.file_.blob_;
    blob.objectURL = blob.objectURL || URL.createObjectURL(blob);
    return blob.objectURL;
}

In FileWriter.write a new blob is created already, so it would remove the temporary variable, and would not be accidentally be written to the datastore.

@ebidel
Copy link
Owner Author

ebidel commented Oct 16, 2014

I like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants