Open Bug 979227 Opened 10 years ago Updated 2 years ago

IndexedDB and createObjectUrl for download

Categories

(Core :: DOM: Core & HTML, enhancement, P5)

27 Branch
x86
macOS
enhancement

Tracking

()

People

(Reporter: guypaskar, Unassigned)

Details

(Keywords: APIchange)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20140212131424

Steps to reproduce:

Wrote a file into indexedDB using file handle API. After the file was fully written to IndexedDB I wanted to download the file using href and download attribute. File was not downloaded.


Actual results:

The call for myFileHandler.getFile() makes the locked file open for read and download. Now, if you want to use the file in <img> that is Ok since this is done sync. But when I attach the file to an anchor using href and then simulate a click to download, nothing happens. Since click is asynchronous in Firefox the locked getFile ends and the lockedFile gets locked again and therefore nothing happens


Expected results:

We should be able to use a locked file to download a file. Either making click synchronous op or creating a function createObjectUrl to indexedDB (or better yet to a file handler like in file system API) since many implementers will want to use indexedDB and for file downloads I think this is an important feature that is missing.
Component: Untriaged → File Handling
Keywords: APIchange, feature
Current workaround (suggested by Jan Varga):

var form = document.createElement('form');
form.action = URL.createObjectURL(mySnapshot);
document.body.appendChild(form);
form.submit();

Problem is that this download a blob and I can't control the name - it looks like someone is downloading some **** file instead of what he wanted to download.

Does anyone have any idea on a different workaround that I can use? It would be very helpful.

Thanks,
Guy
I debuged this a little bit and it seems that there is:
nsDocShell::OnLinkClick() and nsDocShell::OnLinkClickSync()
and the anchor element calls the former.

OnLinkClick() just creates a new runnable which then calls OnLinkClickSync()
OnLinkClickSync() ends up calling blob->GetInternalStream() which is essential for not auto closing the LockedFile associated with it (LockedFile is like the auto closing transactions in IndexedDB).

So, the problem is that blob->GetInternalStream() is not called in the success handler for FileHandle.getFile(), see this example:

<a id="dlink">link</a>

<script>
...
myFileHandle.getFile().onsuccess = function(event) {
  var mySnapshot = event.result; // mySnapshot is a blob
  var dlURL = URL.createObjectURL(mySnapshot);
  var a = $('#dlink');
  a.attr('download', 'test.mp4');
  a.attr('href', dlURL);  
  c.click(); // This should synchronously call blob->getInternalStream() in c++
}
...
</script>

Could we call OnLinkClickSync() in nsContentUtils::TriggerLink() for blob uris ?
Flags: needinfo?(bzbarsky)
Component: File Handling → DOM
Product: Firefox → Core
> Could we call OnLinkClickSync() in nsContentUtils::TriggerLink() for blob uris ?

No; that would be a spec violation, in fact.

Furthermore, I'd expect the code cited above to work even if there is no c.click() call and instead the user just clicks the link, no?  If it doesn't, then something is seriously wrong somewhere (possibly in the IDB spec)!
Flags: needinfo?(bzbarsky)
Well, myFileHandle.getFile().onsuccess receives just a snapshot of the file/blob, so if the success handler doesn't start reading the file/blob (by calling blob->GetInternalStream()) then the lock for the file created internally is released. That means that blob->GetInternalStream() called later will return a failure. When the last chunk of the stream is read or stream->Close() is called or stream object is released, the file lock is released too.

This concept doesn't allow to create several <a href> and let the user to click on it. Actually, it could be done, but the URL would have to be created dynamically in the click handler for <a>, I haven't tested if it actually works this way.
Btw, the c.click() is a typo, it should be a.click()

IDB spec has nothing to do with this, but you can blame FileHandle API instead :)

The future of FileHandle API support in IndexedDB is uncertain (some vendors don't plan to implement it), but FileHandle API is going to be used by FileSystem API, so we should probably try to find a solution for this bug.

Maybe we could add something that explicitly prolongs the lock.
> This concept doesn't allow to create several <a href> and let the user to click on it. 

Why not?  How is it expected to be used, exactly?

What do other UAs do here?
(In reply to Boris Zbarsky [:bz] (reviews will be slow; ask someone else) from comment #5)
> > This concept doesn't allow to create several <a href> and let the user to click on it. 
> 
> Why not?  How is it expected to be used, exactly?

FileHandle API allows writing to files safely, so if there are two tabs accessing the same file, the file shouldn't become corrupted, etc. Auto closing locks (represented by LockedFile objects) are used for this purpose. You can find more info and examples here https://wiki.mozilla.org/WebAPI/FileHandleAPI

FileHandle.getFile() is used to pass "snapshots" of the file to other APIs, like FileReader, IndexedDB, creating a blob uri. 

FileHandle.getFile() internally creates a LockedFile and closes it if the success handler doesn't create a new request to read from the file/snapshot. The request can only be created by calling File::GetInternalStream() in C++. GetInternalStream() is called by other APIs like FileReader, etc. So, if the success handler doesn't cause gecko to call GetInternalStream(), then the LockedFile is closed.

This all means, that one can't generate file uris in advance, they must be generated just in time they are needed. Otherwise the file would have to be locked all the time.

The reason for auto closing locks, is the same we have for transactions in IDB. It's always easy to not handle an error in JS that can lead to not calling lock.close() or transaction.close().

> 
> What do other UAs do here?

There's no such concept (file locking) in other UAs.
Status: UNCONFIRMED → NEW
Ever confirmed: true
> they must be generated just in time they are needed.

The problem is that string URI code all over the platform assumes that doing things async with the string is fine....

This sounds like a similar problem to auto-revoking for blob URLs.  Is that still a thing?

> There's no such concept (file locking) in other UAs.

Do we think they would be willing to implement what we have right now?  It's sounding a bit dubious to me given the restrictions.  :(
(In reply to Boris Zbarsky [:bz] (reviews will be slow; ask someone else) from comment #7)
> > There's no such concept (file locking) in other UAs.
> 
> Do we think they would be willing to implement what we have right now?  It's
> sounding a bit dubious to me given the restrictions.  :(

There's an interest to implement FileSystem API for sure, so I guess yes.
But the current API might need to be modified.

It was several months ago I was part of FileSystem API discussions, something may have changed.
Maybe Jonas has an idea what to do with this bug.
(In reply to Jan Varga [:janv] from comment #4)

> This concept doesn't allow to create several <a href> and let the user to
> click on it. Actually, it could be done, but the URL would have to be
> created dynamically in the click handler for <a>, I haven't tested if it
> actually works this way.
> Btw, the c.click() is a typo, it should be a.click()
  
  This does not work, as discussed in private with Jan, this is the same as not doing it.
  
  Does someone has an idea how to workaround this? 
  right now i'm using form and setting form.action with the blob URI and submitting the form. problem   is that I can't set the name of the download (nor file type) and it feels like downloading a virus or something. Would be happy to hear other ideas.

> Maybe we could add something that explicitly prolongs the lock.
That would be good.
For example, filesystem api (in chrome) has the following method: fileEntry.toURL() and this url is usable everywhere (and anytime)
Any news here?
Is there a workaround to save a file with a name?
I think the solution here is to abandon the auto-revoking behavior that we currently have. Right now our FileHandle implementation will prevent a FileReader from reading a Blob if the read starts asynchronously, likewise it will prevent a fetch from a blob: URL if the fetch starts asynchronously.

This is really good for ensuring no race conditions. However in practice there are too many use cases that doesn't work with that solution.

I think we either need to revoke the Blob/blob:-url only when the file changes. Or we need to create an API which lets you "hold a file open" for a longer time in order to allow responding to some async event.
(In reply to Jonas Sicking (:sicking) from comment #11)
> I think we either need to revoke the Blob/blob:-url only when the file
> changes. Or we need to create an API which lets you "hold a file open" for a
> longer time in order to allow responding to some async event.

Both sound good to me, but I like the latter more.
(In reply to Jonas Sicking (:sicking) from comment #11)

I Agree. Both sounds good - second one gives more flexibility to the developer.

Jonas, Do you have any thoughts on how we can workaround this until an API is created?
Sadly not without copying the file contents by writing the file into IDB again but as a plain Blob rather than a FileHandle.
too bad. If you come up with an idea please let me know.

How can we move forward with this fix then?
Let me try something ...
Obviously, putting alert() to onsuccess handler of getFile() does the trick. It spins the event loop so the asynchronous event dispatched by a.click() gets processed before the blob snapshot gets invalidated.
That's a neat hack :)
But I wouldn't want to prompt an alert to my users for each file being downloaded
This is indeed a nice workaround for now. This also raises a question of if there is any other function that halt all the sync javascript ops like alert but still allow async events to happen like click (something like .pause() ) -Is there anything like that? 

What about a real solution? Any ideas? when should we expect such a change to land?
Any news here? Jan - How can we push this fix forward? 
The Workaround is great and working for now - but not a good solution permanently.

Let me know if I can help push this forward.
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.