Last Comment Bug 714983 - avoid exceptions in file-utils.js when dismissing file picker dialog
: avoid exceptions in file-utils.js when dismissing file picker dialog
Status: VERIFIED FIXED
:
Product: MailNews Core
Classification: Components
Component: Feed Reader (show other bugs)
: Trunk
: x86 Linux
: -- trivial (vote)
: Thunderbird 12.0
Assigned To: :aceman
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-03 15:18 PST by :aceman
Modified: 2012-02-23 08:16 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed patch (2.22 KB, patch)
2012-01-03 15:23 PST, :aceman
no flags Details | Diff | Splinter Review
patch v2 (2.10 KB, patch)
2012-01-04 11:28 PST, :aceman
myk: review+
Details | Diff | Splinter Review

Description :aceman 2012-01-03 15:18:26 PST
When managing feeds subscription, there is Import and Export button. Both of them open a file picker dialog. When that dialog is canceled, there is an exception thrown. I think that is not useful as it is a normal operation so it should be handled gracefully.

The exceptions are:
Error: uncaught exception: [Exception... "Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIFilePicker.file]"  nsresult: "0x804b000a (NS_ERROR_MALFORMED_URI)"  location: "JS frame :: chrome://messenger-newsblog/content/file-utils.js :: pickOpen :: line 207"  data: no]

Error: uncaught exception: [Exception... "Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIFilePicker.file]"  nsresult: "0x804b000a (NS_ERROR_MALFORMED_URI)"  location: "JS frame :: chrome://messenger-newsblog/content/file-utils.js :: pickSaveAs :: line 189"  data: no]
Comment 1 :aceman 2012-01-03 15:23:33 PST
Created attachment 585576 [details] [diff] [review]
proposed patch

I propose this change. In case the dialog is dismissed it seems picker.file is undefined and throws exception. So do not touch that field and do not return it. The parent calling functions properly check rv.reason and do not touch rv.file if the dialog was cancelled. I also do not see why .picker is returned in case of success, I do not see any usage of it. But I didn't change that for now.
If it is better to return all 3 members in all cases, picker.file can be replaced by null in case of Cancel.
Comment 2 Ian Neal 2012-01-03 16:08:31 PST
Comment on attachment 585576 [details] [diff] [review]
proposed patch


>+
>+    if (rv != PICK_CANCEL) {
>         futils.lastSaveAsDir = picker.file.parent;
>+        return {reason: rv, file: picker.file, picker: picker};
>+    } else {
>+        return {reason: rv};
>+    }
> }
>+
>+    if (rv != PICK_CANCEL) {
>         futils.lastOpenDir = picker.file.parent;
>+        return {reason: rv, file: picker.file, picker: picker};
>+    } else {
>+        return {reason: rv};
>+    }
> }
Just as a drive by, in both of the above you don't need to put the second return in the else statement.
Comment 3 :aceman 2012-01-03 23:55:25 PST
Yes, I should have already learned this :)
Comment 4 :aceman 2012-01-04 11:28:15 PST
Created attachment 585819 [details] [diff] [review]
patch v2
Comment 5 Myk Melez [:myk] [@mykmelez] 2012-01-05 19:02:33 PST
Comment on attachment 585819 [details] [diff] [review]
patch v2

Looks good, works as expected, r=myk!
Comment 6 Mark Banner (:standard8) (afk until 26th July) 2012-01-09 00:52:31 PST
Checked in: http://hg.mozilla.org/comm-central/rev/70fa39beb291

Note You need to log in before you can comment on or make changes to this bug.