395956 - Way to handle read-only calendars with the google provider (CAL_IS_READONLY exceptions)

Status: RESOLVED INVALID

(Calendar :: Provider: GData, defect)

Description

[Ingo Müller]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
Build Identifier: 0.2.1 as well as CVS version from 2007-09-12

IMO, the google provider doesn't handle read-only calendars right. When trying to modify, add or delete items from/to a read-only calendar, an ugly error message pops up. Normal read-only calendars don't do that. They just do nothing.

Reproducible: Always

Steps to Reproduce:
1. Create a google calender, create an event or two, then set it to read-only. 
2. Try dragging one of these events, then deleting it.
3. Try adding a new event "the quick way" (click somewhere in the view, hold the mouse button and release it somewhere else a bit further down).
Actual Results:  
Ugly error message with "CAL_IS_READONLY" under "Details"...

Expected Results:  
Behaviour like other read-only calendars: nothing should happen at all.

Comment 1

[Ingo Müller]

Attachment: Throwing the CAL_IS_READONLY exceptions like other calendars do it.

I've never submitted a patch before, so I really hope, that I did everything right. The submitted patch is the output of "C:\Programme\GnuWin32\bin>diff -u -p C:\calGoogleCalendar.orig.js C:\calGoogleCalendar.js > c:\calGoogleCalendar.js.patch". It's a diff against calGoogleCalendar.js I downloaded from http://mxr.mozilla.org/mozilla/source/calendar/providers/gdata/components/calGoogleCalendar.js the 2007-09-12.

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

In initial versions, I checked if the feed was readonly and set the readonly bit on a getItems call. Somewhere about half way down in bug 355117 comment 66, Daniel suggested that I lazily check for readonly.

Furthermore, I think the real issue behind this is how errors are generally reported and how event generation should happen when the calendar is readonly. I think it is correct of the provider to report that the calendar is readonly if it is, your patch would change this.

Bug 379174 will prohibit creating events by dragging when the calendar is readonly. This will get rid of the CAL_IS_READONLY error messages you are getting, since there is no possibility to create events on readonly calendars. Is this an acceptable solution for you? If so, please mark this bug WFM.

In general, you patched correctly. Consider using -u8p as diff options, thats the standard. If you want to do more patches, you could consider checking out the CVS tree and use |cvs diff -u8p| See developer.mozilla.org for details. After you created the patch and attached it to a bug, you should request review. See http://wiki.mozilla.org/Calendar:Module_Ownership to find out who to request review from.

Comment 3

[Ingo Müller]

Thanks for your instructions on patching!

Hmm... I'm not sure, yet. I agree with you, that fixing Bug 379174 will solve this issue, too, at least for Lightning 0.7. I also agree, that the way, that a calendar is read-only is reported, is not very elegant. (Still, it is reported, maybe not through a reporter, but through the exceptions)

Other providers use exceptions, too, to report that they are read-only:

http://mxr.mozilla.org/mozilla/source/calendar/providers/ics/calICSCalendar.js#420
http://mxr.mozilla.org/mozilla/source/calendar/providers/memory/calMemoryCalendar.js#180
http://mxr.mozilla.org/mozilla/source/calendar/providers/storage/calStorageCalendar.js#380

IMO, all providers should behave the same way. If this way is changed from the exceptions to a better way of reporting, or if views stop to try modifying items of a read-only calendar, of course, the google provider has to be changed accordingly. In any way, a provider should not behave in a way, that causes error messages when it's written to although it's read-only.

Maybe, I don't understand the whole thing not well enough, yet. I'm very interested on this subject, though, as I'm developping a provider, too, and have the same problem there. If you're interested, you can get it from https://addons.mozilla.org/en-US/thunderbird/addon/5337.

Comment 4

[Daniel Boelzle [:dbo]]

my 2 cents: IMO all async API of calICalendar should notify errors via listener only, no exceptions; this makes it easier for clients to use that API.

Comment 5

[Ingo Müller]

Alright, then this is a problem of the clients, right? What shall we do then? Maybe I should file a new bug report for the clients? Anyway, if we (you) decide, that the calendars should report via listener only, that they are readonly, then I'd propose to add a comment to calICalendar.idl to clearify the intended behaviour.

Comment 6

[Stefan Sitter [:ssitter]]

See also Bug 313640 that is about improving the current behavior.

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

Daniel, the gdata provider is notifying the listeners with CAL_IS_READONLY. I am catching the exceptions myself and reporting them to the listeners. No exceptions get thrown from getItems/addItem/...

Ingo, I think ssitter's bug is the correct place to fix it for the other providers. I think the calling code should catch exceptions and pass them on to the listener.

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

I think this bug can be closed, since most of the fixing can be done in bug 313640 and bug 379174.

Ingo, any reason to keep it open? If not, please mark INVALID.

Comment 9

[Ingo Müller]

No, you're right, it can be closed.