Open Bug 408739 Opened 17 years ago Updated 3 years ago

clicking already downloading link doesn't pop dlmgr until first download is complete

Categories

(Firefox :: File Handling, defect, P3)

defect

Tracking

()

People

(Reporter: dwitte, Unassigned)

Details

Attachments

(1 file)

str: go to your favorite http download site: http://releases.mozilla.org/pub/mozilla.org/firefox/releases/granparadiso/alpha2/linux-i686/en-US/ click a link to download file (granparadiso-alpha2.tar.bz2) dlmgr pops up and begins downloading close dlmgr click link again get busy spinning cursor and dlmgr doesn't pop up again. we should pop up the dlmgr so the user knows what's going on, otherwise it's thoroughly confusing. ;)
also, favicon in tab continues throbbing, and title is Loading...
I see the same things on win32: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007121216 Minefield/3.0b3pre
Flags: blocking-firefox3?
OS: Linux → All
Hardware: PC → All
Version: unspecified → Trunk
> also, favicon in tab continues throbbing, and title is Loading... if I wait long enough for the first download to finish, what happens next is: 1) I get the alert in the bottom right of my screen (from the alert service) that the download is complete 2) the title of the tab goes back to "Index of /pub/mozilla.org/firefox/releases/granparadiso/alpha2/linux-i686/en-US" 3) the favicon goes back to the right thing (it is no longer the throbber) 4) I get the "Opening granparadiso-alpha2.tar.bz2 dialog
Summary: clicking already downloading link doesn't pop dlmgr → clicking already downloading link doesn't pop dlmgr until first download is complete
No UI feedback is bad enough to block ...
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
(In reply to comment #0) > click a link to download file (granparadiso-alpha2.tar.bz2) > dlmgr pops up and begins downloading > close dlmgr > click link again > get busy spinning cursor and dlmgr doesn't pop up again. Closing the download manager isn't a necessary step, but clicking the same link is crucial. It seems like for some reason, we don't make a request if it's the same as one before. That's why the tab's favicon throbber keeps going. The download manager isn't even notified of the second click. Btw, this is exthandler as it pops open an open-with dialog and alt-clicking to download doesn't show this issue.
File Handling per Comment 5
Component: Download Manager → File Handling
Flags: blocking-firefox3+
Product: Firefox → Core
QA Contact: download.manager → file-handling
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Priority: P3 → P2
Someone suggest an assignee?
Flags: tracking1.9+ → blocking1.9+
I can take a look at this.
Assignee: nobody → honzab
Just for record: this bug is also present in FF2. The reason is very simple: for the download URL is created a cache entry that is not valid during life time of the download. Therefor, when you try to download the file the second time (from the same URL) and HTTP channel tries to open a cache entry for it, it fails with NS_ERROR_CACHE_WAIT_FOR_VALIDATION and HTTP channel then waits for OnCacheEntryAvailable call. It is triggered when 1) the download is complete 2) the download is canceled. This is callstack where the HTTP channel opens the cache entry: > necko.dll!nsHttpChannel::OpenCacheEntry(int offline=0x00000000, int * delayed=0x0012ef3c) Line 1426 C++ necko.dll!nsHttpChannel::Connect(int firstTime=0x00000001) Line 285 + 0x10 bytes C++ necko.dll!nsHttpChannel::AsyncOpen(nsIStreamListener * listener=0x05814520, nsISupports * context=0x00000000) Line 3686 + 0xd bytes C++ docshell.dll!nsURILoader::OpenURI(nsIChannel * channel=0x058138e8, int aIsContentPreferred=0x00000001, nsIInterfaceRequestor * aWindowContext=0x04404268) Line 840 + 0x19 bytes C++ docshell.dll!nsDocShell::DoChannelLoad(nsIChannel * aChannel=0x058138e8, nsIURILoader * aURILoader=0x0356cd20, int aBypassClassifier=0x00000000) Line 7486 + 0x41 bytes C++ docshell.dll!nsDocShell::DoURILoad(nsIURI * aURI=0x05814660, nsIURI * aReferrerURI=0x056b1f60, int aSendReferrer=0x00000001, nsISupports * aOwner=0x00000000, const char * aTypeHint=0x0012f584, nsIInputStream * aPostData=0x00000000, nsIInputStream * aHeadersData=0x00000000, int aFirstParty=0x00000001, nsIDocShell * * aDocShell=0x00000000, nsIRequest * * aRequest=0x0012f4f8, int aIsNewWindowTarget=0x00000000, int aBypassClassifier=0x00000000) Line 7332 + 0x29 bytes C++ docshell.dll!nsDocShell::InternalLoad(nsIURI * aURI=0x05814660, nsIURI * aReferrer=0x056b1f60, nsISupports * aOwner=0x00000000, unsigned int aFlags=0x00000001, const wchar_t * aWindowTarget=0x0012f688, const char * aTypeHint=0x0012f584, nsIInputStream * aPostData=0x00000000, nsIInputStream * aHeadersData=0x00000000, unsigned int aLoadType=0x00200001, nsISHEntry * aSHEntry=0x00000000, int aFirstParty=0x00000001, nsIDocShell * * aDocShell=0x00000000, nsIRequest * * aRequest=0x00000000) Line 7071 + 0x7f bytes C++ docshell.dll!nsWebShell::OnLinkClickSync(nsIContent * aContent=0x056e0cd0, nsIURI * aURI=0x05814660, const wchar_t * aTargetSpec=0x058147a8, nsIInputStream * aPostDataStream=0x00000000, nsIInputStream * aHeadersDataStream=0x00000000, nsIDocShell * * aDocShell=0x00000000, nsIRequest * * aRequest=0x00000000) Line 891 + 0x6a bytes C++ This must be catch somewhere at the level of URI loader or some like (I would appreciate some advice here). We have to pop-up the download manager from there (if not prohibit by user prefs). I will submit a draft patch soon.
Status: NEW → ASSIGNED
The download manager wouldn't pop up until the user selects what to do with the file (open with dialog). So potentially we would need to change the behavior of clicking links that are already open instead of waiting for the cache entry. ?
"already open" means files that are currently being downloaded or those that are already downloaded and potentially present on disk (or/and in the cache)? What you suggest actually means to override the action which user expects to be done when file is downloaded? Problem is that (as I understand) we cannot open the dialog until the channel loads the mime type (or part of content) of the URL. This means that HTTP channel must do the request first but it is blocked with async open of the cache entry.
Well, it depends on what it should do. Clicking the link a second time should... 1) just open the download manager 2) download a second copy ?
Attached patch Draft 1Splinter Review
Please just check this draft solution. It pop-ups the download manager with the item focused. This ignores the preference not to pop-up download manager. I will test it further more.
Attachment #307507 - Flags: review?(edilee)
The code seems reasonable if we want to make the download manager show the item when clicking the link a second time. Not sure if we want to bind the docshell to the toolkit download manager though. Also, we should probably define what's the expected behavior. The patch would make the second click cause the download manager to open as well as not cause a second download to start (even after the first finishes). I suppose we do want the download manager to show when clicking the link, so the question is if there should be another download to start concurrently?
docshell can in no way depend on toolkit fyi
Comment on attachment 307507 [details] [diff] [review] Draft 1 The patch was just an example. Personally I also don't like to call download manager from such a low level code. To sum, there are 3 possibilities: 1. Show the download manager (as the patch does) 2. Ask user what to do and change handle action on the current download 3. Ask user what to do and start another download #1 is very simple just do it more clear way. IMHO I would expect such behavior, because if I want to change the action on the file I can do it after it has been downloaded #2 is more complicated because we must bypass the cache (not to wait for the cache item) or remember the content type on the URI (what might be asynchronous, dependent on the content, in case of e.g. pictures w/o mime type header). #3 means the same (bypass the cache) and actually run/instantiate another one (a concurrent one). As far as I am familiar with the cache code this is probably impossible. As short/middle therm solution I would vote for #1.
Attachment #307507 - Flags: review?(edilee)
If we want solution #1 then I become to conclusion we probably need some new hook in nsDocShell that would override click on link/link load that is already in progress (has been downloading). There is currently no mechanism how to gracefully bypass content load and channel creation (what we must avoid to fix the bad UI response - waiting for the cache item). Any suggestions? I would vote for new observer event like "on-uri-load" taking nsISupportsBoolean that could be set TRUE by observers to bypass the load.
I vote for comment 18 - taking this off the blocking list...
Flags: blocking1.9+ → blocking1.9-
Or even WONTFIX ?
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
Product: Core → Firefox
Version: Trunk → unspecified
Moving to p3 because no activity for at least 1 year(s). See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3

Mass-removing myself from cc; search for 12b9dfe4-ece3-40dc-8d23-60e179f64ac1 or any reasonable part thereof, to mass-delete these notifications (and sorry!)

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: