443006 - 'File>Save Page As' doesn't work - no dialog box is seen

Status: RESOLVED WORKSFORME

(Toolkit :: Downloads API, defect)

Description

[levisa]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0

System seems to do nothing when File>Save Page As or Ctrl-S is accessed. I know that a dialog box is supposed to pop up, but it doesn't.

Reproducible: Always

Steps to Reproduce:
1. Go to any URL
2. Access (click on) File
3. Click on Save Page As or hit Ctrl-S
Actual Results:  
Nothing, no dialog box is seen. A couple of fast hour-glass appearances and then nothing.

Expected Results:  
A dialog box showing possible folders in which to save the page file pops up.

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Do you have any extensions (add-ons) installed?  Have you tried in Safe Mode?  Also, are there any exceptions in Tools | Error Console?

Comment 2

[levisa]

Attachment: The debug and stack trace file.

This is the file generated by WinDbg for the bug. The final few lines are the stack trace. They follow the kp line.

Comment 3

[levisa]

There are quite a few extensions installed. I have not tried it in Safe Mode.

Error Console: 
Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDownloadManager.userDownloadsDirectory]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://global/content/contentAreaUtils.js :: getTargetFile :: line 439"  data: no]

Comment 4

[Martijn Wargers (dead)]

Thanks for that info.
Apparently, you get a js error here:
http://lxr.mozilla.org/seamonkey/source/toolkit/content/contentAreaUtils.js#477

It seems like this:
http://lxr.mozilla.org/seamonkey/source/toolkit/components/downloads/src/nsDownloadManager.cpp#1255
is causing the javascript error somewhere.

It would be helpful, if you could find out if this is caused by one of the extensions you have installed.

Comment 5

[Shawn Wilsher :sdwilsh]

Try reseting the following preferences in about:config:
browser.download.folderList
browser.download.dir

Comment 6

[levisa]

Resetting the two preferences in about:config appears to have fixed the problem. Will it reoccur the next time that I save a page to my hard drive?

Comment 7

[Martijn Wargers (dead)]

Reporter, what were the values of those preferences?
I'm wondering how you got in such a state.
It seems to me Firefox should not give this issue, even if the value of those preferences are wrong. Or may it should, but in that case, it should be impossible to get weird values for those preferences.

Comment 8

[levisa]

I don't remember the values, but they didn't look like anything weird.  The directory referenced was present.  I agree that Firefox should not give this issue.

Comment 9

[Martijn Wargers (dead)]

If there is a way of reproducing this, then I can confirm this bug.

Comment 10

[levisa]

I assume that you have tried inserting some three or four level folder as a value for the browser.download.folderList

Comment 11

[Martijn Wargers (dead)]

Yes, I've tried all kinds of stuff, I didn't get a way to reproduce.

Comment 12

[levisa]

After the fix, I couldn't get it to fail either.  I don't know why.  I guess it is a wait and see whether either I or someone else experience a failure and reports it. Then you can have the person report the values of the browser.download.folderList and the browser.download.dir before the fix is made.

Comment 13

[WADA:World Anti-bad-Duping Agency]

Same exception with different reason code can be re-produced with read-only directory in browser.download.dir, with Fx 3.0.1 on MS Win-XP SP3 with no extention execpt LiveHTTPHeaders.
> Error: uncaught exception: [Exception... "Component returned failure
> code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)
> [nsIDownloadManager.userDownloadsDirectory]"
> nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  <== due to permittion
> location: "JS frame :: chrome://global/content/contentAreaUtils.js :: getTargetFile :: line 439" data: no]

(Step to re-prodece)
(0) C:\WRITABLE : directory with write permission
    C:\READONLY : directory with read-only permission
    non-exsistent-X : directry which doesn't exist
(1) browser.download.useDownloadDir=false
    browser.download.lastDir = C:\WRITABLE\non-existent-A
    browser.download.dir     = C:\READONLY\non-existent-B
    "Save As" => Above error in error consolde, and no file picker dialog
(2) browser.download.useDownloadDir=false
    browser.download.lastDir = C:\WRITABLE\non-existent-A
    browser.download.dir     = C:\WRITABLE\non-existent-B <= changed
    "Save As" => C:\WRITABLE\non-existent-B was created,
                 file picker dialog with C:\WRITABLE\non-existent-B appeared,
                 even though browser.download.useDownloadDir is not true.

Fall back seems to have been changed to following.
> If browser.download.lastDir is not found, Fx 3 falls back to browser.download.dir.
> If browser.download.dir doesn't exist, Fx 3 tries to create it.

If above X:\READONLY\non-existent-B is an unmouted volume such as USB drive, same exception with different reason code seems to occur.
(This is a case reported to a forum in japan.)

Comment 14

[WADA:World Anti-bad-Duping Agency]

Similar exception was reported to a forum.
  - browser.download.dir = USB drive
    The USB drive is automatically powered-off after one hour of no activity.
Reporter says that following message was issued(and needless to say "no file picker dialog") when "Save Image As" was executed after one hour or more since last successful download to the USB drive.
> uncaught exception: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND)
> [nsIDownloadManager.userDownloadsDirectory]" nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"
> location: "JS frame :: chrome://global/content/contentAreaUtils.js :: getTargetFile :: line 439" data: no]

Confirming, because three incidents of "same exception at same location with different failure code" are already available in this bug.

Comment 15

[tech_scholar]

I've experienced this failure as well. Most recently, my relevant settings were:
browser.download.folderList = 2
browser.download.dir = L:\  where L: is a USB stick that is no longer present.

Shawn Wilsher's fix (of resetting those preferences) worked, and I was unable to reproduce the bug, even when I restored the preferences to their original pre-fix settings.

It appears that this thread has found the circumstances that initially produce the bug. (comments #13 and #14.) How hard, I wonder, will it be to find and fix the problematic code? :-)

Comment 16

[WADA:World Anti-bad-Duping Agency]

(In reply to comment #15)

I think your "Shawn Wilsher's fix" is patch for Bug 429827. Right?
As I wrote Bug 429827 Comment #59, problem of Comment #13 case on MS Win was completely resolved by patch for Bug 429827.
(I forgot to post comment to this bug about "Shawn Wilsher's fix"...)

To bug opener & all problem reporters:

Can you reproduce your problem with latest-trunk?
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
> http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-trunk/
> (If MS Win user, use win32.zip build. You can test with UNZIP only.)

Comment 18

[Caleb]

I've recently stumbled into another variation of this problem.

I've installed Firefox on a work computer, whose "My Documents" folder seemed to be on a network, and was read-only. The "Downloads" subfolder was also missing.

By default, Firefox 3.6.6 automatically saves all downloads to the "Downloads" folder. The problem with this is that whenever I try to change that setting an exception is raised:
>Error: uncaught exception: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIDownloadManager.defaultDownloadsDirectory]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: chrome://browser/content/preferences/main.js :: anonymous :: line 353"  data: no]

Because of that execption "browser.download.folderList" remains set to "1" so it always tries to auto-save to that non-existing folder.

None of the Save As dialogs work (that is, they never appear). File > Save Page As does nothing, and when the download dialog appears, clicking "Save" dismisses the dialog and then nothing happens. Clicking "Open", on the other hand, starts the download and opens the file.

I guess this bug could be triggered in many ways, but what is common to them all is a non-existent, inaccessible download folder.

Comment 19

[Pierre Ynard]

The code to choose the download directory, in promiseTargetFile() in
toolkit/content/contentAreaUtils.js, tries three steps:

1/ Try Downloads.getPreferredDownloadsDirectory()
2/ Check browser.download.lastDir
3/ Default to ~/Desktop

It is my understanding that despite failure of none of three steps
being fatal to saving a file, each of them can fail by throwing an
uncaught exception, aborting the whole saving process and in particular
preventing the Save As dialog from opening, with no error message.

1/ can fail in safe mode with a clean, new profile. This would be
the topic of this bug. I can see at least two ways it can fail:

 - filesystem error when trying to create ~/Downloads (read-only
   filesystem, unsufficient permissions, no space left, I/O error...).
   For example, on brand new profile:

> Unix error 30 during operation makeDir on file /home/linkfanel/Downloads (Read-only file system)

 - missing ~/Desktop directory; clean profile,
   browser.download.folderList = 0:

> [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/DownloadIntegration.jsm :: DI_getDirectory :: line 744"  data: no]

2/ can fail if browser.download.lastDir contains invalid data, as
described in related Bug 1258797. At least, that one will be fixed by
resetting preferences.

As far as I can tell, 3/ would throw too if ~/Desktop is missing, but
currently it's actually dead code, since 1/ will never gracefully fail
but always either return an existing directory path, or throw and abort.


I am providing a patch to fix 1/, and so fix this 8 years old bug, which
is arguably the worst part of it. With the asynchronous calls in 2/, I'm
not quite sure how to write it exactly, but again it's a case of trivial
try-catch wrapping. My patch will expose 3/ - it won't be dead code
anymore - but it will still throw if ~/Desktop is missing. It's trivial
to put a try-catch too, but this is already the fallback default, so
what to do then? Does dir have to be initialized for the file picker to
open, can it be null or empty? Can it fallback to the home directory
instead? That one shouldn't be missing. Remember that this is only a
default path passed to the file picker, it doesn't even have to be
valid.

Comment 20

[Pierre Ynard]

Attachment: Patch: Bug 443006 - Handle getPreferredDownloadsDirectory() exception

Patch 100% untested, but trivial concept

Comment 21

[:Paolo Amadini]

(In reply to Pierre Ynard from comment #20)
> Patch 100% untested, but trivial concept

Thanks a lot for the patch! I'll take a closer look at comment 19 and the rest of the bug in the next few days. In the meantime, can you just test the code to see if this runs correctly and solves the issue?

Comment 22

[Pierre Ynard]

I'd have to build firefox to test it, wouldn't I? Sorry, but I'm not doing that. If there's a lighter way like that js file is somewhere I can directly edit then yeah maybe.

Comment 23

[:Paolo Amadini]

If you're concerned about the builds being slow, you may try artifact builds:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds

They are quite similar to directly editing a JS file, except that the "./mach build" command will copy and install the JS file from your source tree for you. Once you have a Mercurial checkout, which I guess you already have since you've already created a patch, it should be relatively simple for you to do an artifact build.

In fact your code inspection skills are great, but part of our job as Firefox developers is also testing our own code changes, and a build environment allows fast iteration and consistent results. Comment 19 is probably on the right track, but we might actually want to move the exception handling to a different area of the code - I can't be sure until I have reviewed the code in detail. When you start modifying multiple files, things become complex pretty soon, and copying each change manually is sub-optimal.

Comment 24

[Pierre Ynard]

Any news on this? Did you get to look at the code more in depth?

I don't especially feel like being an active Firefox developer myself and I'm not even going to look into running any toolchain, but the issues are fairly well described and easily reproducible by anyone. I looked into it and I believe that this would be the right place to handle these exceptions; it seems to me that stuff in DownloadIntegration.jsm should be allowed to throw. But that's just my opinion.

Comment 25

[:Paolo Amadini]

Comment on attachment 8755641 [details] [diff] [review]
Patch: Bug 443006 - Handle getPreferredDownloadsDirectory() exception

We've been busy with our Mozilla All Hands meeting last week, and there are some higher priority follow-ups I have to handle this week. I'll take a look at this feedback request after those are done.

Comment 26

[:Paolo Amadini]

(In reply to Pierre Ynard from comment #19)
> 1/ can fail in safe mode with a clean, new profile. This would be
> the topic of this bug. I can see at least two ways it can fail:
> 
>  - filesystem error when trying to create ~/Downloads
>  - missing ~/Desktop directory

Pierre, thanks for your patience. I took some time today to look at the steps to reproduce you described in comment 19, as well as previous comments on this bug, and concluded that the issue you describe is different from the issue for which this bug was originally filed. The original issue described in comment 0 is likely solved for the original reporter.

The issue you describe is different, and to avoid confusion it needs a new bug with a clear title describing what you'd like to see fixed. If I understand correctly, this is limited to problems with ~/Downloads and ~/Desktop, because the other issue you mentioned seems to be bug 1258797 for which apparently Gijs already described the best solution, which is different from this one.

For the issues with ~/Downloads and ~/Desktop, I have to assume you needed to adopt very special, technical steps to replicate them on your system in the first place. So I don't think this is a high priority to fix, unless there are easier steps to reproduce.

If the above severity evaluation is correct, we might still accept a fully fledged patch on the new bug with an automated test case attached to it, with the understanding that whoever takes the task will have to do the full development cycle and have the required experience, and at least for now I'll not be able to mentor someone on that low priority bug.

Comment 27

[Martijn Wargers (dead)]

Comment 19 reads like there is no interest in this bug.