Closed Bug 328618 Opened 18 years ago Closed 16 years ago

failure when creating calendar objects does not display an error dialog

Categories

(Calendar :: Provider: CalDAV, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dmosedale, Assigned: browning)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
Summary: failure when creating calendars does not display an error dialog → failure when creating calendar objects does not display an error dialog
Whiteboard: [cal relnote]
Flags: blocking0.3?
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.
Flags: blocking0.3? → blocking0.3-
Whiteboard: [cal relnote]
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.
Attachment #291042 - Flags: review?(daniel.boelzle)
Assignee: nobody → browning
Version: Trunk → unspecified
Status: NEW → ASSIGNED
Bruno, IMHO we should use the error announcer UI for all errors. What do you think?
> 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.
Attachment #291042 - Attachment is obsolete: true
Attachment #294396 - Flags: review?(daniel.boelzle)
Attachment #291042 - Flags: review?(daniel.boelzle)
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.
(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 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!
Attachment #294396 - Flags: review?(daniel.boelzle) → review+
Checked in on HEAD and MOZILLA_1_8_BRANCH => FIXED.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.8
You need to log in before you can comment on or make changes to this bug.