When trying to "save link as..." or "save as.." from an URL that has a very slow server, nothing seems to happen. Usually a user will think he missed the context menu option and try again, agin nothing is happening. Then, minutes again, suddenly the save dialog pops up. It seems the dialog is only shown after a reply from the slow server is received. This is very bad, because it is utterly confusing. If the reply is really needed to initialize the dialog (I doubt it), at least pop up an interim dialog with disabled entry fields and stuff greyed out, showing what is happening.

Reporter: Please always specify which "Build ID" you're using, as found in the title bar of the Mozilla window.

build 2002010721/linux - however i believe this problem already exists for some time.

This is a consequence of us fetching the http headers to figure out the content type (and possible content-disposition). There may be other bugs that are essentially the same. We need to see what happens when the link is bogus, also.

this is especially cumbersome in the case an image that is already displayed should be saved: in that case, the image is already there, but save as still seems to contact the server and then refech it instead of just storing whatever is in the cache.

methinks i've been seeing this with today's bits --definitely on all platforms [and mozilla.org appears slow as well].

We could/should find a better way of finding content type for certain types of object, but I'm not likely to get to that before this release.

why do we need the content type if the user specified save as? (i'm assuming this bug is focused on cache misses and not items where we should be copying from cache. i'm assuming there's another bug for the fact that we use a network link when there's a displayed version of the object on the current page)

We don't need the content-type. we need the content-disposition... We should be doing two things: 1) Using the cache 2) Using HEAD, not GET. We currently do neither; the current solution is a kludge. We should be opening our own http channel instead of having webbrowserpersist do it.

Patch to use nsIURIChecker, which does the right thing with HEAD, checking response status, and so forth. Just tested this and this makes us not do a fetch at all for Save Page As and Save Image As. For Save Link As we act more sanely (do a HEAD request instead of GET, so it's much faster).

Akkana, Darin, would you look over my changes to nsIURIChecker? Akkana, what should the load flags be for composer? I left it at LOAD_NORMAL for now, since that's what the old behavior was.... Unfuturing since we have a working patch; this should be retriaged...

This one makes us not crash on GC following Save As. :) AsyncCheckURI returned a ref to the linkChecker but did not addref it.... Also, hold on to the link checker as a property on the header sniffer, just in case.

Comment on attachment 67737 [details] [diff] [review] Patch v 1.1 >Index: xpfe/communicator/resources/content/contentAreaUtils.js >+ this.linkChecker = Components.classes["@mozilla.org/network/urichecker;1"] >+ .createInstance().QueryInterface(Components.interfaces.nsIURIChecker); the explicit call to QueryInterface can be replaced with: .createInstance(Components.interfaces.nsIURIChecker); however, i'm not exactly sure that this is preferred style :-/ patch seems fine to me.. sr=darin

-> 1.0 so I can review bz's patch. We need both the content-type AND the content-disposition (although I see patch 1.1 maintains that).. content-type is used to prefill the filter list with an appropriate set of file types.

Comment on attachment 67737 [details] [diff] [review] Patch v 1.1 Looks good to me. Thanks for catching that missing addref!

If this fixes the problem, I hope we can get it in sooner than 1.0. I hit this all the time ... sometimes the dialog never does come up, sometimes it's several minutes (long after I've given up and gone to a terminal window to run a manual ftp), sometimes I try a second time and then both dialogs come up within seconds of each other.

What happens in the case of a bogus url, or some kind of error occurs while fetching it? It looks like it will just fail silently. I know there's currently issues with error handling in general, but it seems like in this case we should give the user some notification before we go ahead and prompt for a save-to file name. Trust me, we've ignored error handling while downloading far too often and for far too long. I say we fix that bug while we're making it faster.

Indeed, that's no good.... I'll see what I can come up with in the way of error handling.

So I'm seeing two options: 1) Bail on sniffer failures. Put up an alert telling the user the url is "not reachable" or some such. I don't like this approach because I feel that the sniffer can fail while the data is in fact saveable (eg see the hack the sniffer does for 404 responses; maybe there are other such hacks that are needed). 2) Proceed on sniffer failures. Get the content type from the document object if we have a document object, otherwise try the uri extension, otherwise have an empty type calling into foundHeaderInfo. Then if saving _really_ fails handle the error in the persistance object. I personally prefer solution #2, attaching a patch that implements it.

This is untested, unfortunately, since my build is refusing to start... will try to test it tonight.

OK, now it's tested. This tries to get a content type by all the available means till it runs out of options. Makes sure to always put up the filepicker.

Thanks for finding the addref problem! Can we get this reviewed and checked in soon? Composer needs it for link checker and publishing work.

I could land the URIChecker/composer changes now (they have sr=darin, r=akkana,bbaetz) and leave the actual Save As changes until Bill has reviewed them. Would that work?

Comment on attachment 68071 [details] [diff] [review] Patch v 1.3 r=cmanske on the composer part. Tested and works great!

Boris: Sounds good to me!

*** Bug 124105 has been marked as a duplicate of this bug. ***

r=law For the contentAreaUtils.js changes. One thing, though. It looks like we passing too few arguments to saveInternal when called from saveDocument. I think that's why sometimes documents aren't being saved from out of the cache. It also explains the funky "||" in the line at http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/contentAreaUtils.js#200. I'm not thrilled that you're passing the buck on actually handling the errors. But that's only because I'll have to take care of it over in bug 27609 :-).

Well... that should lead to more consistent error-handling, no? :)

Attachment 68071 [details] [diff] checked in. People should retest with builds that include this fix and see how it is...

Oh, it would be consistent anyhow, since I would just cut/paste the same code wherever else it was needed :-).

Cool. Save As dialog appears shortly after the Save Link As command. Not as fast as NS4 (delay is still measurable) but this delay is small enough. So there's no impression that the command doesn't work (as it was before). Build 2002020803, Win98. Didn't tested on Win2K, somehow I think the delay will be signficantly larger there...

Note that if we get _really_ desperate we can put in a timer to fire after some time (1sec?). If we have not heard from the network by then, just give up and guess something... Just a thought if it turns out that it's still taking too long..

Just a thought: what is the worst case? How long will it wait for a server that doesnt respond? A timeout of 1 second sounds reasonable, one does not expect a delay between the save-as request and the dialog.

At the moment the worst case is still whatever the default network timeout is (whicis far too long for this). Hence my suggestion.

regarding this perf on w2k, I see it is much faster now with the patch but not a large noticable delay that is slow to bring up; Boris, if you think you need to tweak the perf on that, I'd be all for it. But, it may be more helpful on a slow system.. maybe add this to the open window/performance tracking tests. I'd say, as of right now the patch makes it much better on w2k, still a small delay but not bad at all. I'm running P3-733 256meg ram w/16384 for disk & memory cache in Moz. -Dennis

so, bz. What is left to be done with this bug?

I say we resolve this and file a separate bug for the timeout thing if/when we decide to do it.... Acting accordingly. :)

The comments at the end of this bug suggest that there was another bug filed to cover the same problem reported here, but don't mention the bug number. This is still as big a problem as ever -- I often wait several minutes between when I choose "Save link as" from the menu and when the dialog actually pops up. Reopening. If the issue is now tracked in another bug, please post the number here.

check to see if you are most likely seeing bug 128927.

Bug 133732 filed.

nsbeta1+ per Nav triage team, adt3

->1.0.1/RTM per Nav triage team. Let's reconsider this after we get beta feedback.

Mass moving nsbeta1+/adt3 bugs assigned to Navigator team engineers out to target milestone 1.0.1. Please confine your attentions to driving down our list of TM 1.0 bugs for beta. Better to help, debug, or test one of them than fix one of these.

resolving fixed again, since the new bug is filed on further work on this stuff.

Before patch 1.3 can be landed on the branch I need to know who sr'ed it, and who r'ed the non contentAreaUtils.js parts. Boris?

Ben, this landed on Feb 7, before there was a branch. The state of this code is identical on trunk and branch at the moment.

Oh, in this case I'll go ahead and mark this bug fixed1.0.0 then. If there's other work that needs to go on the branch/trunk that can be handled in the other bugs mentioned.