Cannot dismiss alarm on single overridden instance of recurring item

VERIFIED FIXED in 0.9

Status

Calendar
Alarms
VERIFIED FIXED
11 years ago
6 years ago

People

(Reporter: dbo, Assigned: dbo)

Tracking

unspecified
Bug Flags:
wanted-calendar0.9 +
in-testsuite ?

Details

Attachments

(2 attachments)

(Assignee)

Description

11 years ago
1. Create a recurring event.
2. Define an alarm on a *single* instance (only) and let it fire.
=> alarm cannot be dismissed.

The current alarm code always saves the last ack stamp at the *parent* item. Due to the inherent saving of X-MOZ-LASTACK inside the VALARM component, you cannot save X-MOZ-LASTACK without an alarm defined.
Duplicate of this bug: 401731
Flags: wanted-calendar0.8?
(Assignee)

Updated

11 years ago
Flags: wanted-calendar0.8? → wanted-calendar0.8+
(Assignee)

Updated

11 years ago
Duplicate of this bug: 408346
(Assignee)

Updated

11 years ago
Assignee: nobody → daniel.boelzle
Flags: wanted-calendar0.8+
(Assignee)

Updated

11 years ago
Flags: blocking-calendar0.8+
(Assignee)

Comment 3

11 years ago
Going on vacation, I won't have time for this; this is wanted for 0.8 though.
Assignee: daniel.boelzle → nobody
Flags: wanted-calendar0.8+
Flags: blocking-calendar0.8-
Flags: blocking-calendar0.8+
Duplicate of this bug: 424871

Updated

10 years ago
Flags: wanted-calendar0.9+
Flags: wanted-calendar0.8+
Flags: blocking-calendar0.8-
(Assignee)

Updated

10 years ago
Duplicate of this bug: 435057
(Assignee)

Comment 6

10 years ago
Created attachment 322799 [details] [diff] [review]
fix
Assignee: nobody → daniel.boelzle
Status: NEW → ASSIGNED
Attachment #322799 - Flags: review?(philipp)
(Assignee)

Comment 7

10 years ago
The fix moves X-MOZ-LASTACK property from the inner VALARM to the VEVENT/VTODO component.
What happens to existing alarms with the change? I fear that this will force everyone upgrading from 0.8 to 0.9 to dismiss their already dismissed alarms.

Also, do we really need to force the lastack timezone to be utc? Are there cases where this might cause a 1 hr offset on a DST boundary or such? If we do need to force this, then I'd suggest either throwing something if the timezone is not UTC, or just converting the date to UTC. 
(Assignee)

Comment 9

10 years ago
(In reply to comment #8)
> What happens to existing alarms with the change? I fear that this will force
> everyone upgrading from 0.8 to 0.9 to dismiss their already dismissed alarms.
Yes I know, this might happen for recent alarms (on ics calendars, not storage). But I really don't think we should move over the old alarm lastAcks. This adds more code and cycles on parsing for little outcome: users just have to dismiss their recent reminders once again.

> Also, do we really need to force the lastack timezone to be utc? Are there
> cases where this might cause a 1 hr offset on a DST boundary or such? If we do
> need to force this, then I'd suggest either throwing something if the timezone
> is not UTC, or just converting the date to UTC. 
The lastAck stamp is saved as ical string (either floating or UTC). I think documenting this at calIItemBase is OK for now, since there's only code storing UTC, thus no bug yet. However, we might consider ensuring this implementing getters and setters, then I'd prefer your second options (coercion to UTC).
(In reply to comment #9)
> (In reply to comment #8)
> > What happens to existing alarms with the change? I fear that this will force
> > everyone upgrading from 0.8 to 0.9 to dismiss their already dismissed alarms.
> Yes I know, this might happen for recent alarms (on ics calendars, not
> storage). But I really don't think we should move over the old alarm lastAcks.
> This adds more code and cycles on parsing for little outcome: users just have
> to dismiss their recent reminders once again.
I know, being backward compatible is a bitch. But you remember how users complained after their calendars were not visible after the update, and there it was also "just checking the checkbox again".

> 
> > Also, do we really need to force the lastack timezone to be utc? Are there
> > cases where this might cause a 1 hr offset on a DST boundary or such? If we do
> > need to force this, then I'd suggest either throwing something if the timezone
> > is not UTC, or just converting the date to UTC. 
> The lastAck stamp is saved as ical string (either floating or UTC). I think
> documenting this at calIItemBase is OK for now, since there's only code storing
> UTC, thus no bug yet. However, we might consider ensuring this implementing
> getters and setters, then I'd prefer your second options (coercion to UTC).
Yes, please.

(Assignee)

Comment 11

10 years ago
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > What happens to existing alarms with the change? I fear that this will force
> > > everyone upgrading from 0.8 to 0.9 to dismiss their already dismissed alarms.
> > Yes I know, this might happen for recent alarms (on ics calendars, not
> > storage). But I really don't think we should move over the old alarm lastAcks.
> > This adds more code and cycles on parsing for little outcome: users just have
> > to dismiss their recent reminders once again.
> I know, being backward compatible is a bitch. But you remember how users
> complained after their calendars were not visible after the update, and there
> it was also "just checking the checkbox again".
I think that's slightly different, because users were missing their calendars at that time (and IMO another indication that the visibility check isn't recognized as such, but that's another story). W.r.t. alarms they get the recent ones from ics again, and I think that's OK and don't reason to further complicate the code.

> > > Also, do we really need to force the lastack timezone to be utc? Are there
> > > cases where this might cause a 1 hr offset on a DST boundary or such? If we do
> > > need to force this, then I'd suggest either throwing something if the timezone
> > > is not UTC, or just converting the date to UTC. 
> > The lastAck stamp is saved as ical string (either floating or UTC). I think
> > documenting this at calIItemBase is OK for now, since there's only code storing
> > UTC, thus no bug yet. However, we might consider ensuring this implementing
> > getters and setters, then I'd prefer your second options (coercion to UTC).
> Yes, please.
Hey, that's just been a suggestion I've done while reading the code; it's off-topic to this bug :-) I think I've done an improvement in this field by adding a simple comment to IDL.
(In reply to comment #11)
> I think that's slightly different, because users were missing their calendars
> at that time (and IMO another indication that the visibility check isn't
> recognized as such, but that's another story). W.r.t. alarms they get the
> recent ones from ics again, and I think that's OK and don't reason to further
> complicate the code.
Since the alarms are only from the past weeks, I guess its ok. I didn't think about that. We should add a release note though.


Keywords: relnote
Comment on attachment 322799 [details] [diff] [review]
fix

One thing that comes to mind is that the currently unused calIAlarm interface also allows setting lastAck on the VALARM. If this is to be changed also, then setting lastAck depends on the item being set, or the base item needs to keep the method to set the alarm last ack.

Great bonus points for also fixing calIAlarm, although I won't r- based on that.

r=philipp
Attachment #322799 - Flags: review?(philipp) → review+
(Assignee)

Comment 14

10 years ago
(In reply to comment #10)
> > UTC, thus no bug yet. However, we might consider ensuring this implementing
> > getters and setters, then I'd prefer your second options (coercion to UTC).
> Yes, please.
I did that.

(In reply to comment #13)
> Great bonus points for also fixing calIAlarm, although I won't r- based on
> that.
I think we should fix the yet unused calAlarm in another bug; I've opened bug 437196.

Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.9

Comment 15

10 years ago
Checked issue with nightly build 2008073118 and sunbird 20080731 -> VERIFIED
Status: RESOLVED → VERIFIED
Daniel, I need a good description of the open issue with regards to the reappearing old alarms for the release notes (see comment 11 and comment 23). 

Any suggestions?
(Assignee)

Comment 17

10 years ago
Maybe: "It may happen that you have to dismiss previous alarms set on recurring instances again.". But to be honest, I don't think it's really relnote-worthy.
(In reply to comment #17)
> But to be honest, I don't think it's really relnote-worthy.

I take your word for it :)

Keywords: relnote

Comment 19

10 years ago
Created attachment 336855 [details]
Special Event Instance Alarm Example ICS

I think there might still be an issue related to this (as of build 2008090218).  I have a recurring event with an alarm (Mondays at 10am).  An instance of this event has been moved to the following day (Tuesday), and the alarm for that instance will not dismiss/dismiss all.  I have to close the reminder box using the X button, but the reminder keeps popping up a few minutes later (I'm assuming it's snoozing). I've even tried deleting the instance, editing the ICS file to delete the EXDATE under the event, and then creating the special instance (moving it to Tuesday) again in Lightning just to make sure it didn't get corrupted somehow.  There's other special instances earlier than this one if that matters.  I'm attaching a sample ICS file that has the problem.

Comment 20

10 years ago
I get very similar problems dismissing alarms on a shared gcal using google calendar add on and private XML address.  My recurring event reminders can't be dismissed or snoozed  ....  Never used a bug service, what should I do?

Updated

10 years ago
Flags: blocking-calendar0.9+
(Assignee)

Comment 21

10 years ago
File a bug against gdata-provider, but please leave the blocking flag to release drivers.
Flags: blocking-calendar0.9+

Comment 22

10 years ago
Just to add some info i found.
Since Dismiss, Dismiss All, Snooze don't do anything and don't produce and error, I tried to turn off the reminder for the individual task.

I clicked details on the reminder pop up.  I edit all instances of recurring item.
I set reminder to none. and save and close.

Now the error console had this:

Error: aItem.calendar has no properties
Source File: chrome://calendar/content/calendar-item-editing.js
Error: aItem.calendar has no properties
Source File: chrome://calendar/content/calendar-item-editing.js
Line: 476
Error: [Exception... "'[JavaScript Error: "aItem.calendar has no properties" {file: "chrome://calendar/content/calendar-item-editing.js" line: 476}]' when calling method: [calIOperationListener::onOperationComplete]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///D:/Program%20Files/Calendar/components/calItemModule.js -> file:///D:/Program%20Files/Calendar/js/calTransactionManager.js :: cT_onOperationComplete :: line 144"  data: yes]
Line: 476


How should I report this bug?
(In reply to comment #20)
This seems to be a known issue for shared Google Calendars, see Bug 424185. According to the last comment it might be fixed using Lightning 0.9 and Provider for Google Calendar 0.5 (not yet released.)
Flags: in-testsuite?

Comment 24

6 years ago
This issue still affects me - been extremely annoying for a long while now on multiple OS's (Mac & Linux)

Mac 10.6.8
Ubuntu 12.04.1 64-bit
TB 15.0
Lightning 1.7
Provider 0.16

I subscribe to a number of my Cal's (across several Google accounts); they're shared privately server-side, but *none* of them are either cached or read-only.

I've looked into some suggestions found online, but I think this is essentially a long-running client-side bug.
http://www.consumingexperience.com/2009/01/thunderbird-reminders-keep-popping-up.html
You need to log in before you can comment on or make changes to this bug.