Closed
Bug 129614
Opened 23 years ago
Closed 23 years ago
low space in target directory: no error dialog, incomplete download
Categories
(Core Graveyard :: File Handling, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.0.2
People
(Reporter: bugzilla, Assigned: law)
References
Details
(Whiteboard: [adt2 rtm],custrtm- [verified on trunk])
Attachments
(2 files, 2 obsolete files)
42.54 KB,
text/html
|
Details | |
28.93 KB,
patch
|
bzbarsky
:
review+
jud
:
superreview+
jud
:
approval+
|
Details | Diff | Splinter Review |
found while verifying scenarios for bug 27609, on linux rh7.2 using
2002.03.06.07 comm bits.
1. unlike bug 129604, /tmp is no longer full. instead, i fill up another
partition where i'll be saving the file --in my case /export/builds/foopy
2. click to download the following 35M file:
ftp://ftp.mozilla.org/pub/mozilla/nightly/2002-03-07-08-trunk/mozilla-source.tar.gz
3. in the file picker, select /export/builds/foopy as the target location.
results: no error dialog appears. the download progress dialog showed
completion, too. however, when looking at the contents of /export/builds/foopy,
the file there was incomplete --definitely less than the expected 35M. moreover,
the salted file persisted in /tmp even after quitting.
later on, i'll try this out by saving to a floppy disk.
btw, not a problem on mac 10.1.3 when i tried saving to a Zip disk which didn't
have enough space.
will test win2k shortly...
Reporter | ||
Comment 1•23 years ago
|
||
this is also a problem on win2k, using 2002.03.06.06 comm verif bits. i tried
saving the same 35M file to a floppy and a ramdisk --both lacking the space.
there was no error dlg, and the download progress dlg appeared done.
difference in results: no file is saved at all, and no lingering temp file in
C:\WINNT\Temp...
Keywords: nsbeta1
OS: Linux → All
I think this could be due to insufficient error handling when moving/copying the
downloaded file from temp to final destination. There may already be a bug on that.
Reporter | ||
Updated•23 years ago
|
Summary: low space: no error dialog, incomplete download → low space in target directory: no error dialog, incomplete download
Comment 3•23 years ago
|
||
We really should ensure adequate space exists on the destination at the time it
is chosen, and preferably copy the bits directly there. Using an intermediate
file just introduces lots more failure cases, and takes extra time and disk space.
Comment 4•23 years ago
|
||
nsbeta1+ per Nav triage team, should at least notify user.
Comment 6•23 years ago
|
||
sairuh: isn't this a duipe of bug 115201 ?
Comment 7•23 years ago
|
||
Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.) Thanks!
Changing from nsbeta1+ back to nsbeta1 (nominated only), so the triage team can
decide.
Comment 9•23 years ago
|
||
Nav triage team: nsbeta1+/adt2
Updated•23 years ago
|
Whiteboard: [nav2][adt2] → [adt2]
Comment 10•23 years ago
|
||
I have noticed this with 1.0rc1. If the disk is completly full before the
download the download appears to be successful but there is no file on the disk.
This was when I was saving to a Samba share that was completely full. This was
quite confusing for awhile because I could not figure out why the file was not
there.
Assignee | ||
Comment 11•23 years ago
|
||
This seems to fix most problems, still needs more testing before it's done. I
have to upload it here so I can do other work on these files.
Assignee | ||
Comment 12•23 years ago
|
||
Revised patch including changes to download manager.
Ready for review...
Attachment #82157 -
Attachment is obsolete: true
Assignee | ||
Comment 13•23 years ago
|
||
Please consult this for explanation of the fix.
Comment 14•23 years ago
|
||
So... the alert we put up in the "really rare case" has no parent. Does this
not mean that it is non-modal? Should we not use mWindowContext as the parent?
(QI to nsIInterfaceRequestor and GetInterface(nsIDOMWindowInternal) as
nsHelperAppDlg.js does).
Other than that, this looks great. r=bzbarsky if you address the alert-parent
issue.
Comment 15•23 years ago
|
||
Comment on attachment 82747 [details] [diff] [review]
patch v2.0
sr=blake
Thanks for those comments. That really facilitates review.
A few minor nits:
// Exception mans file doesn't exist; launch is not allowed.
Please fix that typo.
And please don't wrap the condition of that |if| with spaces in
nsDownloadManager.cpp.
Attachment #82747 -
Flags: superreview+
Assignee | ||
Comment 16•23 years ago
|
||
I added this line to get the parent from mWindowContext:
+ nsCOMPtr<nsIDOMWindow> parent(do_GetInterface(mWindowContext));
This required a bit more work because SendStatusChange wasn't a member function
and to get at mWindowContext I had to make it one). I also tweaked some of the
error handling code to make sure Cancel() is called whenever an error occurs
(that was necessary to get the download manager to mark failed downloads as
such rather than "completed").
Attachment #82747 -
Attachment is obsolete: true
Comment 17•23 years ago
|
||
Comment on attachment 83824 [details] [diff] [review]
patch v3.0, incorporating Boris's suggestion
r=bzbarsky
looks good. Don't forget to check in Mac build changes if needed when you land
this...
Attachment #83824 -
Flags: review+
Assignee | ||
Comment 18•23 years ago
|
||
fixed
Assignee | ||
Comment 19•23 years ago
|
||
no mac build changes required
fixed
Updated•23 years ago
|
Whiteboard: [adt2] → [adt2 rtm]
Keywords: mozilla1.0.1
Comment 20•22 years ago
|
||
Comment on attachment 83824 [details] [diff] [review]
patch v3.0, incorporating Boris's suggestion
assuming carry over of sr. please slap me if I'm wrong. also approving patch.
nice comments!
Attachment #83824 -
Flags: superreview+
Attachment #83824 -
Flags: approval+
Comment 21•22 years ago
|
||
please checkin to the 1.0.1 branch ASAP. once there please remove the
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
Comment 22•22 years ago
|
||
Mass removing adt1.0.0, and adding adt1.0.1 because, we are now on 1.0.1.
Keywords: adt1.0.0
Comment 23•22 years ago
|
||
sairuh, could you verify this on the trunk?
Reporter | ||
Comment 24•22 years ago
|
||
couldn't get to this till now (was on vacation during comment 23).
on win2k (2002.06.13.08 commercial trunk), this works fine now. tested both the
context menu ("save link target") as well as via helper app dlg (single-click
downloadable link).
on mac 10.1.5 (2002.06.13.08 commercial trunk), this also works (same tests as
with win2k). the only hitch here is that it took a while for the error dlg to
actually appear. ie, for the "save link target" test, 16% was downloaded before
an error dlg appeared. and for the helper app dlg, progress went as far as 100%
before the error appeared.
the Mac OS X behavior is not very friendly, but i'd say the particular issue of
getting no dlg at all is fixed on the trunk.
Whiteboard: [adt2 rtm],custrtm- → [adt2 rtm],custrtm- [verified on trunk]
Reporter | ||
Comment 25•22 years ago
|
||
retested with today's mac os x trunk build: didn't seem to take as long when
using "save link target as" --so filed bug 152392 for the "waiting till 100%..."
issue.
Status: RESOLVED → VERIFIED
Comment 26•22 years ago
|
||
adding adt1.0.1+. Please get drivers approval again before checking into the
branch.
Comment 27•22 years ago
|
||
If/when this lands on the branch, bug 147142 should land as well.
Comment 28•22 years ago
|
||
law: was this patch checked into the branch? if yes, pls replace "mozilla1.0.1+"
with the "fixed1.0.1" keyword.
Comment 29•22 years ago
|
||
adt1.0.1- per the ADT. let's get it in the next release.
Comment 30•22 years ago
|
||
Retargeting since no one is pushed this for 1.0.1 (no one re-asked for approval).
Keywords: mozilla1.0.1+ → mozilla1.0.1
Target Milestone: mozilla1.0 → mozilla1.0.2
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•