Closed Bug 415742 Opened 16 years ago Closed 16 years ago

Alarms on occurrences cannot be dismissed

Categories

(Calendar :: Provider: GData, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Fallen, Assigned: Fallen)

Details

(Whiteboard: [gdata-0.4])

Attachments

(1 file)

Currently, the provider only stores one X-MOZ-LASTACK extended property on the Google Servers. Other properties to dismiss single occurrences are not stored.
(In reply to bug 397030 comment #37)
> Philipp, I don't fully understand what the issues are, but I can try to take a
> look and see if I can help.  I'm looking at the code in calGoogleUtils.js, and
> what I gather from your comment is, you're using gd:extendedData properties to
> store and retrieve X-MOZ-LASTACK and X-MOZ-SNOOZE-TIME values, correct?
Yes


> these correspond to the most-recent alarm you've acknowledged and the time you
> last snoozed an alarm?
No, not quite. This extended property gives me the lastack and snooze time for the master item. Recurring items have not been taken into account here.

I guess a solution would be to enumerate through all X-Props and check for X-MOZ-LASTACK-nnnnnnnnnn properties, which are the lastacks of the single occurrences. I haven't done this since I dont really want to enumerate through all X-Props, I'd like to be able to retrieve the lastack ones specifically. The same goes for snooze-time.

Things might get easier when multiple alarms are implemented, since this throws over the whole alarm system.


>  What "other X-Props" are lastack-related, and is it a
> failing of the GData API that you can't manipulate them properly?
I can add anything using a gd:extendedProperty

> Looking at
> how you handle single-instance versus recurring events, the recurring case
> never checks gd::when or gd::reminder -- according to the google docs about
> its API, that's because they're not part of a gd::recurrence?
On a recurring item, I retrieve the start date from the <gd:recurrence> tag. it is encoded as an ics DTSTART property. Since I havent taken care of reminders and it seemed non-trivial for me to dismiss recurring alarms, I didnt parse the reminder tags for recurring events. Therefore Im quite supprised that alarms show up at all.

Thanks for the details.  The alarms, as I described in the 7-step procedure in bug 397030 comment #35, happen when I've just created an alarm for the master event, and haven't yet dismissed anything.  That may be why they show up at all :)

The following are rambles under the assumptions that
1. Lightning continues to have multiple alarms unimplemented
2. It's very cheap to repeatedly query for a single X-Prop (e.g. just a single fetch from Google, but multiple XML queries on the returned data; that's fine...)

Idea 1, assuming some interesting functionality from XML: I'm not familiar with the syntax being used in calGoogleUtils.js for manipulating xml data in XMLEntryToItem -- I think I've traced it to jsxml.c/h, but I haven't had time to try to learn that code yet.  Is there anything in that library for doing xpath queries?  Or use a jQuery-like library (unless there are licensing issues?)?  Then your problems with enumerating through all X-Props goes away completely and easily.

Idea 2, assuming no interesting functionality from XML: What if you changed the semantics of X-MOZ-LASTACK to mean "the most-recent alarm not yet dismissed."  Then if you have alarms 1, 2 and 3 (on M, W and F, let's say) and X-MOZ-LASTACK=2 (or Wednesday's date/time, I'm just staying abstract for now), then you've dismissed alarms 1 and 2, and when Friday rolls around, alarm 3 pops up and on dismissal, X-MOZ-LASTACK updates to 3.

But clearly that's untenable, since you might not always dismiss a consecutive sequence of alarms -- if all three pop up on Friday and you dismiss 2, then 1 shouldn't go away...  By analogy, the not-yet-dismissed alarms in the past of X-MOZ-LASTACK are like exceptions to a recurring event -- assume that all alarms have been dismissed, but mark the ones that haven't.  Doing so in a trivial way would lead you back to the enumerating for X-MOZ-LASTACK-nnnnnnnnn problem, so do it as a linked list instead:  Create an X-MOZ-LASTNOTACK => <datetime1> property, that contains the date/time of the latest alarm not yet acknowledged.  Then create X-MOZ-LASTNOTACK-<datetime1> => <datetime2> that tells you the next unacknowledged alarm, etc.  Finally, X-MOZ-LASTNOTACK-<datetimeN> => "none" terminates the exception list.

Disadvantages: for a linear number of unacknowledged, nonconsecutive alarms, this has at least linear cost (depending on how cheap it is to query for these properties; I sure hope you don't have to fetch data from Google every time! :)

Advantages: My guess is that people won't often have a long tail of unacknowledged alarms -- if there are a lot, then it's probably a backlog from being away on vacation or something and so they may just Dismiss All instead; otherwise, this is a fairly simple way to keep track of multiple alarms without explicit multiple-alarm support.
Enumerating the X-Props from the Google XML feed is quite easy. This is just a matter of 

for each (var lastack in xml.gd::extendedProperty.(@value.substr(0,14) == "X-MOZ-LASTACK-")) { .. } // Or similar, didn't test this.

The problem is rather enumerating the props on the calIItemBase. My fears could be  unfounded though, this might turn out to be cheaper than I think.

Note that X-MOZ-LASTACK is used by the alarm system, so I dont really have much freedom regarding the use.

I think its worth a shot to see what happens if the properties are enumerated.

The syntax to access the xml data is called e4x. There are some tutorials on about.com I believe. It is quite simple:

foo.bar.baz.@value gives you "oof" from:

var foo = <foo><bar><baz value="oof"/></bar></foo>;
Assignee: nobody → blerner
Ben, are you still working on this? It seems that X-MOZ-LASTACK is only saved once, but X-MOZ-SNOOZE-TIME-nnnnnnn is saved for each snoozed occurrence.

There also seem to be some problems with Google's recurring item alarms, see a recent newsgroup post of mine.

I have a patch almost ready (though untested), don't want to take over if you have already gotten reasonably far though.
sorry -- I've been laid up with the flu for the past few weeks, and haven't gotten anywhere.  If you have a patch together, you're far ahead of me :)  (Also, this is the first bug I've worked on, not merely commented on, so I'd been on the learning curve before I got sick.)  I can try testing your patch though over the next few days, if that'd help you...
Ben, feel free to test this patch. Review will happen some time after the 8th, and I hope to get this in for the 0.4 release.
Assignee: blerner → philipp
Status: NEW → ASSIGNED
Attachment #307006 - Flags: review?(daniel.boelzle)
Ok.  I took the latest code in CVS, patched it with the above, and have it running in TB2.0.0.12/Lightning 2008030419 (I'd updated to the latest nightly, but it's broken today...)

Experience so far: due to whatever I'd done with prior versions of Gdata, I had to recreate all my recurring events -- even if I went into Google Calendar, I couldn't create alarms for them, it just got snarled up.  So my exact procedure:
1) In an existing google calendar, delete all recurring events.
2) In Lightning/Gdata, recreate recurring events.
2a) For a test, I created a dummy event, starting on Monday and repeating three times (until today).  Give it a five-minute alarm.
3) Refresh Google Calendar, and sure enough, Google thinks the events have alarms.
4) Lightning pops up an alert for every single prior instance of the recurring events.
5) Dismiss the oldest event (note: not dismiss-all).  Lightning/gdata dismisses *all* prior alarms.  Not sure if this is right or not, but I'm willing to overlook it as a startup problem, or as not-yet-implemented.
6) We'll see tomorrow whether the recurring events then (whose expired alarms I dismissed in step 5) give me alarms tomorrow as expected.  That's the main test...
6a) Encouragingly, the alarm set in 2a just fired, so that's good news :)

Semi-relatedly, I have a one-time exception (Tuesday@2pm) to one of the recurring events (Tuesdays@11:00).  Google calendar thinks it does *not* have an alarm, but Gdata *does*.  Hmm...
This morning, Lightning/gdata did fire a single alarm for the new instance of this morning's recurring event, so that worked.

Scenario: I subscribe to my calendars on both my laptop and my school desktop.  The alarm fired on the desktop, and was dismissed on the desktop.  Later I turned my laptop on.  The alarm never fired on my laptop.

I have another recurring event later this afternoon.  I'll leave both machines running, and see if the alarm fires on both, and what happens if I dismiss it on just one machine.
Update: both machines raised the alarm.  I dismissed it on the laptop, and ensured I resynched the calendars with the server.  The desktop then failed to dismiss the alarm -- dismiss and dismiss-all both didn't work.  Closing the alerts window succeeded, but I won't know until the next alarm is raised whether this alarm was dismissed.
Comment on attachment 307006 [details] [diff] [review]
Add Snooze and dismiss for recurring alarms - v1

r=dbo
Attachment #307006 - Flags: review?(daniel.boelzle) → review+
Ben, thank you very much for testing. Most behavior you described is not a problem of the gdata provider, but more a general problem and covered in other bugs.

I believe the problem you describe in comment 9 is the result of a conflict, but you say you made sure the calendars are were synched. Any error console messages?

I think this patch is a good step in the right direction, if this shows up again, please file a new bug.

Checked in on HEAD and MOZILLA_1_8_BRANCH and SUNBIRD_0_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [gdata-cvs]
I forgot to check the error log :-\  The next time it comes up, I'll be sure to do so.  As a simple workaround for now, closing the alarm window doesn't ever cause the dismissed-on-other-machine alarms to be re-raised, and the next instance of the event does raise alarms.

Let me know if there are other bugs/patches to help test...I'd be glad to help.
Whiteboard: [gdata-cvs] → [gdata-0.4]
You need to log in before you can comment on or make changes to this bug.