Lack of error handling in nsIFileWidget calls

VERIFIED FIXED

Status

P3
normal
VERIFIED FIXED
20 years ago
2 years ago

People

(Reporter: sfraser_bugs, Assigned: rods)

Tracking

Trunk
All
Mac System 8.5

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

20 years ago
nsIFileWidget is used in a number of places for opening/saving files. In the
two cases that I've notice so far, there is absolutely no error checking
to test whether the component manager actually managed to create a
file widget. So the code looks like:

  nsIFileWidget* fileWidget;
  nsComponentManager::CreateInstance(kCFileWidgetCID, nsnull, kIFileWidgetIID,
(void**)&fileWidget);

  fileWidget->SetFilterList(5, titles, filters);

This is lack of error handling is appalling; in a componentized world such as
ours, who knows what trivial user error could result in the component manager
being unable to create an instance of something. Check return values, please!

Updated

20 years ago
Status: NEW → ASSIGNED

Comment 1

20 years ago
If error-checking code isn't in the woods, and a call doesn't fail, does anybody
not hear it?

The code is broken; I'll fix it.  But let the record show I didn't write the
original version (I did copy the bad version into some of my code though, so I
feel worse about that).

Updated

20 years ago
Assignee: law → rods
Status: ASSIGNED → NEW
Component: XP Toolkit/Widgets → other

Comment 2

20 years ago
Rod, this appears to be your code ...
(Assignee)

Comment 3

20 years ago
I copied the code from elsewhere also, and feel as bad a Bill.... I'll track
down the origin and fix it, and promise to do a better job when copied other
people's code.

Bill, did you fix this or do you want me to?
(Assignee)

Updated

20 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 4

20 years ago
I have fixed it, I check it in when the tree opens.
(Assignee)

Updated

20 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 20 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

20 years ago
Added the error handle in OnSave(), and OpenWindow()

Updated

20 years ago
Status: RESOLVED → VERIFIED

Comment 6

20 years ago
i did a search on the source tree and found two files that appeared
to have said error checking updated -- (cvs version 1.89)

nsBrowserAppCore.cpp line 1297 nsFileDownloadDialog::OnSave()
nsBrowserAppCore.cpp line 1594 nsFileDownloadDialog::OpenWindow()

marking verified

Updated

20 years ago
Status: VERIFIED → REOPENED

Comment 7

20 years ago
Reopening. The error handling (and leak removal) went in to OpenWindow(), but not
into OnSave().
(Assignee)

Updated

20 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 8

20 years ago
I guess that is a matter of opinion, I will change it to check the error code,
but the current code checks to see if the filewidget is not null, so it is error
checking and it is safe.
(Assignee)

Updated

20 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 20 years ago20 years ago
(Assignee)

Comment 9

20 years ago
The OnSave method got removed by law.

Comment 10

20 years ago
so can i mark this bug VERIFIED?

Comment 11

20 years ago
Yes, you can mark this verified; the code mentioned here is obsolete.

Updated

20 years ago
Status: RESOLVED → VERIFIED

Comment 12

20 years ago
thanks!  VERIFIED it is.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.