Last Comment Bug 328618 - failure when creating calendar objects does not display an error dialog
: failure when creating calendar objects does not display an error dialog
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Provider: CalDAV (show other bugs)
: unspecified
: All All
: -- normal (vote)
: 0.8
Assigned To: Bruno Browning
:
:
Mentors:
Depends on: 328617
Blocks:
  Show dependency treegraph
 
Reported: 2006-02-25 20:06 PST by Dan Mosedale (:dmose)
Modified: 2008-01-17 10:42 PST (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
show alert when creating provider fails (3.27 KB, patch)
2007-12-01 20:47 PST, Bruno Browning
no flags Details | Diff | Splinter Review
patch version 2, using calErrorPrompt.xul (4.83 KB, patch)
2007-12-22 18:22 PST, Bruno Browning
dbo.moz: review+
Details | Diff | Splinter Review

Description Dan Mosedale (:dmose) 2006-02-25 20:06:11 PST
While working on bug 327971, it became clear calendars get created by the first call to calCalendarManager.getCalendars(), which eats any failures (bug 328617 field).  Since the first caller of getCalendars is the error announcer, we need to have some other bit of UI to actually display any errors that happen there to the user.
Comment 1 Joey Minta 2006-08-02 15:28:39 PDT
Will accept a patch, but for 0.3 we can live with this.  Failure to create a calendar is likely a problem in our code and not something the user can solve on his end.
Comment 2 Bruno Browning 2007-12-01 20:47:01 PST
Created attachment 291042 [details] [diff] [review]
show alert when creating provider fails

I don't think that displaying the actual error is of any use here at all, so this just displays a "something went wrong" db. The actual error, such as it is, can still be seen in the console.
Standard alert db but one new user-visible string here; I'm not sure whether I need to request ui-review or not, nor if so from whom.
Comment 3 Daniel Boelzle [:dbo] 2007-12-10 09:50:03 PST
Bruno, IMHO we should use the error announcer UI for all errors. What do you think?
Comment 4 Bruno Browning 2007-12-22 18:22:48 PST
Created attachment 294396 [details] [diff] [review]
patch version 2, using calErrorPrompt.xul

> Bruno, IMHO we should use the error announcer UI for all errors. What do you
> think?

I think there's a good reason I don't do UI-reviews. :)  This version of the patch uses the same UI as the error announcer, though not the error announcer itself, obviously.
Comment 5 Daniel Boelzle [:dbo] 2007-12-28 07:54:34 PST
Comment on attachment 294396 [details] [diff] [review]
patch version 2, using calErrorPrompt.xul

>+        try {
>+            var calendar = Components.classes["@mozilla.org/calendar/calendar;1?type="
>+                + type].createInstance(Components.interfaces.calICalendar);
>+            calendar.uri = uri;
>+            return calendar;
>+        } catch (ex) {

Hmm, thinking about this I doubt that we get the real errors, because async providers usually don't do much in the uri setters, but run into trouble when accessing the resources.
Comment 6 Bruno Browning 2007-12-30 22:46:16 PST
(In reply to comment #5)
> Hmm, thinking about this I doubt that we get the real errors, because async
> providers usually don't do much in the uri setters, but run into trouble when
> accessing the resources.

True. The errors we'll catch here are quite generic and uninformative; they'll look something like:

Error: Can't create calendar for 3 (calday, http://leyta:8008/calendars/users/bruno/calendar/): TypeError: Components.classes["@mozilla.org/calendar/calendar;1?type=" + type] has no properties

and in my experience the underlying cause is always a bug rather than something the users can do anything about. The issue, as I see it, is that currently when we fail to create a calendar at startup there is no notice to the user other than something like the above in the error console (where most users won't know to look): the program seems to have started normally until the user notices that a calendar is mysteriously missing from the list. Displaying an error here is mostly to demonstrate that we're not so lame that we don't notice when calendars go missing.

There's nothing particular to async calendars here; I'm not sure why the bug was filed against the CalDAV provider. The problem is rare enough that we could consider WONTFIXing the bug. But catching the error and displaying /something/ seems a reasonable thing to do.


Comment 7 Daniel Boelzle [:dbo] 2008-01-17 09:56:21 PST
Comment on attachment 294396 [details] [diff] [review]
patch version 2, using calErrorPrompt.xul

r=dbo, only small nits:

>+        try {
>+            var calendar = Components.classes["@mozilla.org/calendar/calendar;1?type="
>+                + type].createInstance(Components.interfaces.calICalendar);
>+            calendar.uri = uri;
>+            return calendar;
>+        } catch (ex) {
an additional ASSERT(false, ex) logs into the js console.

.createInstance(Components.interfaces.nsIDialogParamBlock);
>+            var sbs = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                            .getService(Components.interfaces.nsIStringBundleService);
>+            var props = sbs.createBundle("chrome://calendar/locale/calendar.properties");
above lines are obsolete

>+unableToCreateProvider=An error was encountered preparing the calendar located at %1$S for use. It will not be available before restarting.
I shortened that to: "An error was encountered preparing the calendar located at %1$S for use. It will not be available."
since we don't know if it'll ever come back :)

other than that I debitrotted the patch and checked it in, because of the upcoming string freeze on monday.

thanks for the patch, Bruno!
Comment 8 Daniel Boelzle [:dbo] 2008-01-17 09:58:22 PST
Checked in on HEAD and MOZILLA_1_8_BRANCH => FIXED.

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