The default bug view has changed. See this FAQ.

avoid exceptions in file-utils.js when dismissing file picker dialog

VERIFIED FIXED in Thunderbird 12.0

Status

MailNews Core
Feed Reader
--
trivial
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

Trunk
Thunderbird 12.0
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.10 KB, patch
myk
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
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]
(Assignee)

Comment 1

5 years ago
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.
Attachment #585576 - Flags: review?(myk)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED

Comment 2

5 years ago
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.
(Assignee)

Comment 3

5 years ago
Yes, I should have already learned this :)
Summary: remove exceptions in file-utils.js when dismissing file picker dialog → avoid exceptions in file-utils.js when dismissing file picker dialog
(Assignee)

Comment 4

5 years ago
Created attachment 585819 [details] [diff] [review]
patch v2
Attachment #585576 - Attachment is obsolete: true
Attachment #585576 - Flags: review?(myk)
Attachment #585819 - Flags: review?(myk)
Comment on attachment 585819 [details] [diff] [review]
patch v2

Looks good, works as expected, r=myk!
Attachment #585819 - Flags: review?(myk) → review+
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/70fa39beb291
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 12.0
(Assignee)

Updated

5 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.