Closed Bug 701659 Opened 13 years ago Closed 13 years ago

Alarms fire but cannot be removed from the Alarm Dialog for occurrences of repeating items starting more than 6 hours in the future

Categories

(Calendar :: Alarms, defect)

Lightning 1.0
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcclain.salem, Assigned: mmecca)

References

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
Build ID: 20111104165243

Steps to reproduce:

Clicked to snooze a reminder of a recurring event by selecting a custom time period from the drop down menu; program doesn't respond immediately so, thinking I didn't click it properly, clicked the green check button again.


Actual results:

Error message: "An error occurred when writing to the calendar Home!" Clicking the "details" button yields this additional message: "Error code: MODIFICATION_FAILED Description: [blank]. Any attempt to redo snooze or dismiss the reminder results in the same error message. When you close the reminder, you get the same message, but the reminder goes away, then it returns in a few minutes or any time another reminder pops up, with the same result. The only way you can get rid of it is to delete the event and start over.


Expected results:

It should accept the snooze the first time you click it. If you click it twice before it has accepted the first click, it should ignore the second click.
OS: All → Windows XP
NEW INFORMATION - It turns out you don't have to click twice to snooze. I have had two more occurrences of this today. I was careful not to click twice. Clicked to snooze once, and the system did not respond. I waited several seconds, still no response, and finally clicked to close the reminder dialog box . . . got the same error message as above.
SAME RESULT if you click "DISMISS". Once you attempt to snooze and (for whatever reason) it does not successfully snooze the reminder, this error appears whatever you do next.
What type of calendar is it? When you restart Thunderbird, before attempting to dismiss or snooze the reminder, is the calendar set to read-only (you can check the setting by double-clicking the calendar name in the calendar list)
The calendar is the default one that was created when I installed Lightning. The only modification I've done is to add the holiday calendar using the method described on the Mozilla site. It is not set to "read only". 

BTW This error only occurs occasionally - most of the time when I click to snooze it works correctly.
Next time you see this error, would you check for calendar related errors in Tools -> Error Console and post what you find?
Confirming.

Steps to reproduce:
1) Create a new event. Set Start date to 12 hours from now, Repeat to "Daily", and Reminder to "1 day before".
2) Click Save and Close. The alarm does not fire.
3) Restart Thunderbird.
4) The alarm fires. Click Dismiss.

Actual Results:
The item is not removed from the Alarm dialog, and the dialog does not close, but the calendar data appears to be modified correctly. Further attempts to close the dialog, or to snooze or dismiss the alarm that is still incorrectly shown results in a MODIFICATION_FAILED error due to the fact that the item has already been modified.
Assignee: nobody → matthew.mecca
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: regression
OS: Windows XP → All
Summary: Persistent error when clicking snooze twice → Alarms fire but cannot be removed from the Alarm Dialog for occurrences of repeating items starting more than 6 hours in the future
Attached patch Fix v1Splinter Review
Makes sure occurrence range on alarm removal covers the same period as the occurrence range on startup.
Attachment #574109 - Flags: review?(philipp)
I posted a screenshot of the error console
Severity: normal → major
Comment on attachment 574109 [details] [diff] [review]
Fix v1

Review of attachment 574109 [details] [diff] [review]:
-----------------------------------------------------------------

r=philipp with comments considered and approval for aurora/beta. Let me know what you think:

::: calendar/base/src/calAlarmService.js
@@ +294,3 @@
>                  } else {
>                      // This is a subsequent search, so we got all the past alarms before
>                      start = this.alarmService.mRangeEnd.clone();

Does the mRangeStart need to be updated in the else case in any way here?

@@ +452,4 @@
>              // We search 1 month in each direction for alarms.  Therefore,
> +            // we need occurrences between initial start date and 1 month from now
> +            let until = nowUTC();
> +            until.month += 1;

So now we no longer use mRangeEnd here, but it looks like its still updated once in a while. Why were we using mRangeEnd before? What is it needed for?
Attachment #574109 - Flags: review?(philipp) → review+
(In reply to Philipp Kewisch [:Fallen] from comment #10)
> 
> ::: calendar/base/src/calAlarmService.js
> @@ +294,3 @@
> >                  } else {
> >                      // This is a subsequent search, so we got all the past alarms before
> >                      start = this.alarmService.mRangeEnd.clone();
> 
> Does the mRangeStart need to be updated in the else case in any way here?

This else case fires when the alarm service is updated every 6 hours. We could push mRangeStart forward 6 hours every update as well, which would offer a slight performance gain over time when expanding occurrences of repeating items when finding alarms. However, doing so would run the risk of this bug occurring from the other side, that is it would be possible to snooze an alarm repeatedly until it is outside of the occurrence range, eventually preventing it from being removed from the dialog. In order to guard against this we would need to search for those old occurrences and remove them every time the alarm service is updated.

I don't think this would be a net performance gain unless Thunderbird is left running for a really long time between restarts. Maybe we could split the difference and do a complete reload of alarms, like at startup, once every day or two, and update mRangeStart only at that point. I think if we decide to do that it should be in followup bug.


> @@ +452,4 @@
> >              // We search 1 month in each direction for alarms.  Therefore,
> > +            // we need occurrences between initial start date and 1 month from now
> > +            let until = nowUTC();
> > +            until.month += 1;
> 
> So now we no longer use mRangeEnd here, but it looks like its still updated
> once in a while. Why were we using mRangeEnd before? What is it needed for?

We still use mRangeEnd to calculate which alarms to set timers on when the alarm service is updated, so that we don't need to create timers days in advance.
Ok, in that case go ahead, given your explanation I don't think we should change anything.

I fear that 1.1b1 build1 was already started, so we'll have to postpone this to 1.1b2 or respin 1.1b1. Approval to push to comm-beta, be sure you are committing to the default branch since tip may be on the release branch.
We have a build2, so this will be part of 1.1b1:

pushed to comm-central changeset f7ba3c3bb06a
pushed to releases/comm-aurora changeset 2e6e0faf665b
pushed to releases/comm-beta changeset 2e6dd05b60dc
pushed to 1.1b1 relbranch on releases/comm-beta changeset e51dd73ebf3a
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.1
You need to log in before you can comment on or make changes to this bug.