Closed
Bug 578096
Opened 14 years ago
Closed 14 years ago
Uploading a file via drag-and-drop and XMLHttpRequest.send(File) locks the file
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b7
Tracking | Status | |
---|---|---|
blocking1.9.2 | --- | needed |
status1.9.2 | --- | .14-fixed |
status1.9.1 | --- | unaffected |
People
(Reporter: lfrenkel, Assigned: khuey)
References
Details
(Keywords: testcase, verified1.9.2, Whiteboard: [should be fixed by 583863])
Attachments
(3 files, 2 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
sicking
:
review+
christian
:
approval1.9.2.14+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.99 Safari/533.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6 Drag-and-drop in conjunction with XMLHttpRequest.send(File) locks the uploaded file. This method works, but after the operation, the uploaded file can't be moved with Windows Explorer. Attempting to move the file results in the following error message: "The action can't be completed because the file is open in Firefox." The file stays locked until Firefox is restarted. Reproducible: Always Steps to Reproduce: 1. Upload a file via drag-and-drop and XMLHttpRequest.send(File) 2. Try moving the file to a different folder with Windows Explorer 3. Observe the error message ""The action can't be completed because the file is open in Firefox" Actual Results: Error message Expected Results: No error message
Comment 1•14 years ago
|
||
See the comments in bug 183689 - it's often caused by the LiveHTTPHeaders or Firebug extensions. see also bug 459384
Updated•14 years ago
|
Severity: critical → normal
Component: Developer Tools → General
QA Contact: developer.tools → general
Comment 2•14 years ago
|
||
As far as understand this report is about the new features introduced in Firefox 3.6, so the older bugs aren't necessarily relevant. lfrenkel: do you have a testcase or could you create one please?
Comment 3•14 years ago
|
||
Ah, yes. This is a bug in the patch for bug 491201. Compare the code in XHR file upload: 2311 // Feed local file input stream into our upload channel 2312 return NS_NewLocalFileInputStream(aResult, internalFile); to the code in form submission file upload: 503 // Get input stream 504 rv = NS_NewLocalFileInputStream(getter_AddRefs(fileStream), 505 aFile, -1, -1, 506 nsIFileInputStream::CLOSE_ON_EOF | 507 nsIFileInputStream::REOPEN_ON_REWIND); XHR needs to do the same, plus probably make sure to explicitly close the stream if the POST doesn't end up actually getting sent (e.g. if the preflight fails or whatever).
Blocks: 491201
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Keywords: testcase-wanted
Comment 4•14 years ago
|
||
Jonas, can you take this? That part about handling the error cases seems to be most up your alley....
Comment 5•14 years ago
|
||
Though... when the stream object is destroyed, it should close itself. So if we're really not releasing the file, that suggests that something is leaking. So I really would like to see a testcase here...
Keywords: testcase-wanted
This problem isn't limited to drag-and-drop. It also happens with <input type="file"> and XMLHttpRequest.send(File). Something like this should do it, where blah.html is can handle the upload. <html> <head> <script> function handleChange() { var file = document.getElementById('file'); var xhr = new XMLHttpRequest(); xhr.open('POST', 'blah.html', true); xhr.send(file.files[0]); } </script> </head> <body> <input type="file" id="file" onchange="handleChange()"> </body> </html>
Comment 7•14 years ago
|
||
Comment 8•14 years ago
|
||
I can't seem to reproduce the bug with the attached testcase on trunk. The file only stays locked until the GC which finalizes the XHR object (and thence its mChannel and thence the file stream). Reporter, do you see the problem with the attached testcase?
Comment 10•14 years ago
|
||
Say the XMLHttpRequest object is not GCed, shouldn't the file still be closed after it's sent or failed to be sent?
Comment 11•14 years ago
|
||
That's what comment 3 is about, yes. I'm just concerned about why the reporter is seeing them not GCed, in addition to the bug we need to fix here.
Comment 12•14 years ago
|
||
Ah, I assumed "The file stays locked until Firefox is restarted." from comment 0 applied to a more complex situation than the testcase (like the Gmail case), where something might be holding on to the XHR object. lfrenkel, do you have a testcase for the situation you described in comment 0, where nothing is supposed to hold the XMLHttpRequest object alive? (Or is it the same testcase from comment 6?) It might be worth filing a separate bug on that.
Comment 13•14 years ago
|
||
Not going to block on this without a reproducible testcase. Please renominate if/when a testcase is available.
blocking2.0: ? → ---
Comment 14•14 years ago
|
||
The attached testcase can be used to reproduce the problem, as currently described in summary, although not the "The file stays locked until Firefox is restarted" part of comment 0. If there is a case when "The file stays locked until Firefox is restarted", it should be filed separately with a testcase. Until then we should definitely fix the issue with the testcase. Re-setting the blocking? flag, since it was requested for the issue in comment 3 (the current summary) originally.
blocking2.0: --- → ?
Keywords: testcase-wanted → testcase
Comment 15•14 years ago
|
||
Yeah, I think this is a must-fix, since it causes files to be locked for arbitrary (from the user's point of view) lengths of time.
Assignee | ||
Comment 18•14 years ago
|
||
(In reply to comment #17) > By making us share the code with form submission? If so, awesome. Yep! :-)
Whiteboard: [should be fixed by 583863]
Assignee | ||
Comment 19•14 years ago
|
||
I just pushed 583863. If somebody could verify that this bug is fixed on or after http://hg.mozilla.org/mozilla-central/rev/af1365b24066 that would be awesome.
Assignee | ||
Comment 20•14 years ago
|
||
Make that http://hg.mozilla.org/mozilla-central/rev/8d846fde08cb. Today's nightly or anything later should be fixed.
Marking this FIXED by Kyles patch in bug 583863. Would still be great to get this confirmed.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 22•14 years ago
|
||
Yes, both this testcase and gmail's uploading (bug 574221) works fine with Mozilla/5.0 (Windows NT 6.0; rv:2.0b6pre) Gecko/20100909 Firefox/4.0b6pre, though I can reproduce on Firefox 3.6.
Target Milestone: --- → mozilla2.0b6
Version: unspecified → Trunk
Assignee | ||
Comment 23•14 years ago
|
||
This should be testable, right? The issue is that the XHR is keeping the nsIFile alive (and thus the file on disk locked) until the XHR is destroyed even though the file is no longer needed after the send. If my understanding is correct, I will write a test for this.
blocking2.0: ? → ---
Flags: in-testsuite?
Assignee | ||
Comment 24•14 years ago
|
||
Rereading the comments, it looks like if we keep the XHR alive we should be able to test this.
Assignee: nobody → khuey
Comment 25•14 years ago
|
||
> The issue is that the XHR is keeping the nsIFile alive
The nsFileInputStream, but yes.
Assignee | ||
Comment 26•14 years ago
|
||
Assignee | ||
Comment 29•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/5a429d9307e4
Flags: in-testsuite? → in-testsuite+
Comment 30•14 years ago
|
||
Seems that this is only a partial fix (tested on 4.0b6). In the GMail test case, the file is locked after uploading, and is released only a few moments after it is done. The file may stay locked even if the tab is closed. In comparison, if the uploaded in the regular way it is released immediately after being uploaded.
Assignee | ||
Comment 35•14 years ago
|
||
We're going to want to backport a fix to 1.9.2. There are a lot of user complaints about this.
status1.9.2:
--- → ?
Comment 36•14 years ago
|
||
It seems like bug 583863 fixes it, which is a pretty major change. Why is it important to block release? Where are we seeing user complaints? Are we able to get a less-risky patch for branch?
blocking1.9.2: --- → ?
Comment 38•14 years ago
|
||
> Where are we seeing user complaints?
We're not. gmail is, though, from Firefox 3.6 users dragging in attachments.
Comment 39•14 years ago
|
||
we're also seeing duplicate bugs filed, those should count as complaints.
status1.9.1:
--- → unaffected
Updated•14 years ago
|
blocking1.9.2: ? → needed
Comment 40•14 years ago
|
||
(In reply to comment #37) > We're definitely going to want a much narrower patch on branch. Do you think we'll have a less invasive patch for the next branch release (likely due mid Feb)?
Assignee | ||
Comment 41•14 years ago
|
||
Yeah, I'm not likely to get to touch this until after my semester finishes, but as long as code freeze for that release is in 2011 I should be able to make that.
Assignee | ||
Comment 42•14 years ago
|
||
Assignee | ||
Comment 43•14 years ago
|
||
Comment on attachment 502699 [details] [diff] [review] XHR locks file longer than necessary. Would nice to get this in by tomorrow's freeze.
Attachment #502699 -
Flags: review?(jonas)
This is too late for Firefox4, we should do it for the next release though.
Comment on attachment 502699 [details] [diff] [review] XHR locks file longer than necessary. Ah, this is for 3.6
Attachment #502699 -
Flags: review?(jonas) → review+
Comment 46•14 years ago
|
||
Comment on attachment 502699 [details] [diff] [review] XHR locks file longer than necessary. a=LegNeato for 1.9.2.14. As you know, code freeze for non-blockers such as this bug is tomorrow evening PST.
Attachment #502699 -
Flags: approval1.9.2.14+
Comment 48•14 years ago
|
||
I can't reproduce this with the attached testcase on 1.9.2.13. Since I see that others have had issues there as well, can someone give me actual steps to reproduce? Also, Kyle, is there a reason why the automated test wasn't checked into 1.9.2 with the fix? Does it need a review?
Whiteboard: [should be fixed by 583863] → [should be fixed by 583863] [qa-examined-192]
Assignee | ||
Comment 49•14 years ago
|
||
I forgot about landing the mochitest, but now that I run the test on a 1.9.2 build from before this change I see that the test does not fail. I'll poke at it a bit more as time permits. In the meantime, hopefully we can verify this through the gmail drag and drop attacher (which seemed to be what most complaints were about) or something else.
Comment 50•14 years ago
|
||
Do you know the steps to reproduce with the gmail drag and drop attacher (I'm unfamiliar with it)?
Comment 51•14 years ago
|
||
Al: see http://gmailblog.blogspot.com/2010/04/drag-and-drop-attachments-onto-messages.html and steps to reproduce from bug 574221.
Comment 52•14 years ago
|
||
Happily, it turns out that gmail does useragent sniffing and turns off this feature in nightlies so I had to override my UA string. :-) That said, I can verify that the problem is present in 1.9.2.13 and fixed in the nightly 1.9.2.14pre build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.14pre) Gecko/20110113 Namoroka/3.6.14pre ( .NET CLR 3.5.30729)).
Keywords: verified1.9.2
Whiteboard: [should be fixed by 583863] [qa-examined-192] → [should be fixed by 583863]
You need to log in
before you can comment on or make changes to this bug.
Description
•