292704 - New alarm service

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Stuart Parmenter]

code to handle alarms in the new calendar backend world

Comment 1

[Stuart Parmenter]

Attachment: calIAlarmService and friends

Comment 2

[Stuart Parmenter]

Attachment: Implement it all _and_ make it work

This patch gets the alarm service up and running.  It observes calendar
registering and unregistering, event adds/deletes/modifies, and makes toast!

'app-startup' is apparently too early for me to use here since it can't
actually get the calStorageProfile stuff at that point needed by
calendarManager's getCalendars... I've wired it in in lightning but I'd rather
it just auto-startup.

This code currently doesn't pop up an alert when the alarm fires... it used to
use alert(), but components can't use that.  It could be made to use nsIPrompt,
but it really needs to bring up a real dialog so you can snooze and whatnot.

Comment 3

[Mike Shaver (:shaver emeritus)]

Can you write, here or in another patch, a design overview of the alarm stuff? 
How you select which timers to set, what happens if you can't find the event
when the timer goes off, what we do for snoozed timers when the user shuts down
before the snooze goes off, etc.?

I think you probably want a category like profile-after-change or some such, for
the post-profile-selection startup trigger.

Comment 4

[Stuart Parmenter]

The way things work is:

At startup, we register an observer with the calendar manager for registering
and unregistering calendars so that we can look for their events as well.  For
each calendar, we then register an observer so that we can monitor
add/modify/delete so that we can adjust timers appropriately.  We then look for
all existing calendars and do a getItems for all events from now until 6 hours
from now and we look for ones with alarms.  If the event has an alarm we create
a timer to fire at that alarm time.  If the event gets deleted, we'll cancel the
timer.  Currently the timer object holds a strong ref to the event so it won't
ever have trouble actually finding the event, but if it gets deleted or modified
we should get rid of that ref when we cancel the timer.  Every 6 hours after the
start we redo the getItems and set up new timers.

All that said, alarms are currently cacluated when they are set in the event
dialog.  When the user snoozes an alarm, it should modify the event for a later
alarm time, which will cause the service to do the right thing.  Things for how
alarms and recurrence work is somewhat up in the air and something we should
discuss further.

I tried using profile-after-change last night, but it didn't seem to get fired.

Comment 5

[Stuart Parmenter]

Attachment: Latest and greatest

This patch fully implements calIAlarmService::shutdown() as well as makes a
first pass at bringing up a very rough alarm dialog

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 182563 [details] [diff] [review]
Latest and greatest

shutdown and startup should go away.

The host app (ltn, sunbird, extension) should register an alarm observer
(nsIObserver is probably sufficient, with the event as the detail) rather than
having the alarm service do anything with the window watcher.

calIAlarmService.idl needs docs.

calAlarmService's constructor looks like it's setting stuff that we would
rather just put on the prototype.

I don't think you want to have a timer per alarm-time, but instead a timer for
the soonest alarm, which causes you to walk the ordered list of alarm-times to
notify the observers of firing alarms.	I could live with that being done in
another patch, after 0.8, though.

I've never seen this "service,@mozilla.org/contractid;1" thing before -- what's
it for?

r-, but you knew you needed to change this up anyway.

Comment 7

[Stuart Parmenter]

Attachment: New dialog, xbl, etc...

I've made the new dialog actually work.  It is being called now from a worse
place than before though.  I'm not sure where the best place to hook up the
observer is.  Thoughts?

Comment 8

[Stuart Parmenter]

Attachment: Finalish patch

I believe this patch only has 2 bugs:
1) it looks for events in a 6 hour period and checks to see if they have alarms
as opposed to looking for alarms in a 6 hour period.  We should address this in
a seperate bug by adding additional stuff to calICalendar (probably extending
getItems)
2) the addDuration code in calIDateTime is broken and causes snooze not to work
as intended (15 minute snooze.)

Additional feature bugs should be filed for:
1) allowing the user to specify how long to snooze for
2) closing the dialog automatically when there are no alarms left in it
3) giving the dialog some pretty style

Comment 9

[Mike Shaver (:shaver emeritus)]

Comment on attachment 182728 [details] [diff] [review]
Finalish patch

r=shaver with comments from f2f review.

Comment 10

[Stefan Sitter [:ssitter]]

Snoozing an alarm in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20051208 Mozilla Sunbird/0.3a1+ results in the following javascript error:

----------------------------------------------------------------------------
Error: [Exception... "Could not convert JavaScript argument arg 0 [calIDateTime.addDuration]"  nsresult: "0x80570009 (NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame :: file:///D:/sunbird/components/calAlarmService.js :: anonymous :: line 195"  data: no]
----------------------------------------------------------------------------

The problem seems to be that snoozeEvent() passes a calIDateTime object to addDuration(); but a calIDuration object is expected.

I just want to know if this is the bug mentioned in comment #8 or if we need a new bug filed:

> 2) the addDuration code in calIDateTime is broken and causes snooze not to
> work as intended (15 minute snooze.)

Comment 11

[Stefan Sitter [:ssitter]]

This patch was checked in long ago, marking as fixed.
Follow up bugs are already filed or will be filed if necessary.