Closed
Bug 54789
Opened 24 years ago
Closed 24 years ago
Crash saving all attachments - GetFolder mode handled incorrectly
Categories
(Core :: XUL, defect, P2)
Tracking
()
VERIFIED
FIXED
People
(Reporter: bryner, Assigned: bryner)
References
Details
(Keywords: crash, platform-parity, Whiteboard: [rtm+ need info])
Attachments
(2 files)
3.58 KB,
patch
|
Details | Diff | Splinter Review | |
3.50 KB,
patch
|
Details | Diff | Splinter Review |
Currently we don't handle the filepicker's GetFolder mode correctly. In particular: 1) we assume there is a filter list. this is wrong, and causes a crash trying to save all attachments in mail. 2) we don't even want to show the filter list in GetFolder mode 3) only folders should be shown in this mode. I have a patch that addresses these problems.
Assignee | ||
Comment 1•24 years ago
|
||
Nominating for RTM based on the fact that this causes a 100% reproduceable crash on a fairly accessible (and useful!) operation.
Assignee | ||
Comment 2•24 years ago
|
||
Assignee | ||
Comment 3•24 years ago
|
||
Assignee | ||
Comment 4•24 years ago
|
||
the patch i just attached is somewhat cleaner and also addresses the fact that if no folder is selected, the current directory should be used.
Comment 5•24 years ago
|
||
r=jag on the second patch, adding keywwords (patch, approval), marking critical.
Comment 6•24 years ago
|
||
rtm+ need info, please remove 'need info' when super-reviewer adds a= to this bug report. Changing summary to reflect user problem rather than underlying cause.
Priority: P3 → P2
Summary: GetFolder mode handled incorrectly → Crash saving all attachments - GetFolder mode handled incorrectly
Whiteboard: FIX IN HAND → [rtm+ need info] FIX IN HAND
Target Milestone: --- → M19
Comment 7•24 years ago
|
||
a=ben
Comment 8•24 years ago
|
||
Removing 'need info', this should be ready to double-plus and land.
Whiteboard: [rtm+ need info] FIX IN HAND → [rtm+] FIX IN HAND
Assignee | ||
Comment 10•24 years ago
|
||
checked in on trunk, waiting for branch to open...
Comment 11•24 years ago
|
||
r=pavlov
Assignee | ||
Comment 12•24 years ago
|
||
checked in on branch
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 14•24 years ago
|
||
*** Bug 55452 has been marked as a duplicate of this bug. ***
Comment 15•24 years ago
|
||
This still happens on today's build 2000101308.
Comment 17•24 years ago
|
||
*** Bug 55452 has been marked as a duplicate of this bug. ***
Comment 18•24 years ago
|
||
*** Bug 56490 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 19•24 years ago
|
||
jeff- Ok, apparently there IS a problem if you try to select a directory via changing TO that directory and hitting OK. Selecting the directory from the list (in the parent directory) works fine for me. Changing to the target directory and hitting OK results in nothing happening. Are you still seeing the crash, or just this problem where the OK button doesn't work?
Status: REOPENED → ASSIGNED
Comment 20•24 years ago
|
||
This bug has re-opened. Clearly the patch that was approved did not heal the problem. Pushing the status back to rtm-need-info, and removing "fix in hand" comment, and patch keyword. Please update with status, and nominate if you have a safe reviewed fix.
Keywords: patch
Whiteboard: [rtm++] FIX IN HAND → [rtm need info]
Comment 21•24 years ago
|
||
Best case resolution for this may be just simple bulletproofing that stops the crash, if one still exists. If the only problem is that the OK button is active when there is no directory selected, then please minus and future this bug.
Whiteboard: [rtm need info] → [rtm+ need info]
Assignee | ||
Comment 22•24 years ago
|
||
I don't believe there is still a crash, at least I can't reproduce one. I'm waiting for jefft to tell me whether he is seeing a crash or not, and how to reproduce it. Neither jag or I can make this crash in today's builds. The problem I *am* seeing, is that if you try to select a folder for the saved attachments by changing TO that directory (as opposed to clicking on the folder from the level above and then clicking OK), the filepicker will not go away. This all boils down to the fact that we weren't clearing the filename textfield on the directory change, so the name of the folder that you changed into is still there. For example: - Suppose I have a directory under my home directory for saved attachments, /home/bryner/attachments. When the filepicker comes up, it will start in /home/bryner. - If I select the attachments directory by clicking once on it in the file list and clicking OK, everything is fine. - But, if I double-click the attachments directory to change to that directory, the textfield will still contain the text "attachments" (since clicking on a file puts its name in the textfield), so clicking OK will try to save in /home/bryner/attachments/attachments, which does not exist, and the filepicker doesn't exit because you haven't selected something valid. Can be worked around by manually clearing the textfield. Note that this problem ONLY manifests itself in the "Select Folder" mode of the filepicker, which is currently only used, to my knowledge, in Save All Attachments. There is an easy, 1-line fix for this to make sure we clear the textfield on chdir. If you think this fix is worth it, we can do a trial checkin on the trunk (after proper reviews and testing), otherwise, ->Future (assuming there is no crash scenario).
Comment 23•24 years ago
|
||
No, it is too late for such minor improvements. Typical case will be to select directory while it is selected. If that is the only problem, then this will turn into rtm-/future.
Comment 24•24 years ago
|
||
The biggest problem I am having with my test is I cannot pick any directory at all. The drop down directory list is empty. Click on ".." button does nothing. Then click on the "Cancel" button it crashed out with the following error messages. Adding myself to the cc list. (I am testing with my daily branch daily build.) can't get parent directory nsWidget::~nsWidget() of toplevel: 32 widgets still exist. nsWidget::~nsWidget() of toplevel: 29 widgets still exist. WEBSHELL- = 7 ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "[JavaScript Warning: "reference to undefined property o.retvals.buttonStatus" {file: "/home/jefftsai/branch/ns/dist/bin/components/nsFilePicker.js" line: 171}]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "JS frame :: /home/jefftsai/branch/ns/dist/bin/components/nsFilePicker.js :: anonymous :: line 171" data: yes] ************************************************************ ###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../dist/include/nsCOMPtr.h, line 649 ###!!! Break: at file ../../dist/include/nsCOMPtr.h, line 649
Assignee | ||
Comment 25•24 years ago
|
||
That sounds EXACTLY like what I fixed a couple of weeks ago. Are you sure you don't have other mods in your tree? Can you try today's branch build from sweetlou?
Comment 26•24 years ago
|
||
I did a grep on my cvsco.log and cvsco-ns.log and I don't have any local modifications. I'll try the release build from sweetlou next.
Comment 27•24 years ago
|
||
The release build does fix the problem. I am puzzled now how come my branch build doesn't. marking the bug fixed.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•