395288 - [Proto] Event dialog: Wrong Reminder entry shown for reminders greater '7 days before the event starts'

Status: VERIFIED FIXED

(Calendar :: General, defect)

Description

[Stefan Sitter [:ssitter]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.7pre) Gecko/20070906 Calendar/0.7pre

Steps To Reproduce:
1. Start Sunbird with clean profile
2. Create new event with custom reminder "8 days before the event starts"
3. Save event and close event dialog. Reopen event dialog.

Actual results:
Reminder item in the Event dialog shows default entry "1 day before". In the Custom Reminder dialog the correct entry "8 days before the event starts" is displayed.

Expected results:
Reminder item in the Event dialog shows entry "Custom..." and the reminder summary.

Note:
A reminder "9 days before the event starts" will be shown as "2 days before".
A reminder "10 days before the event starts" will be shown as "3 days before".
...

A custom reminder "8 days after the event starts" works OK.
A custom reminder "8 days before the event ends" works OK.
A custom reminder "8 days after the event ends" works OK.

Comment 1

[Stefan Sitter [:ssitter]]

Daniel, you reported the same issue in Bug 388094 Comment #6. Reminder entries greater 7 days are probably not very common but if they are used the issue is very visible to the user. Should it block the release?

Comment 2

[Stefan Sitter [:ssitter]]

It works now for reminders using >7 days. I think this was fixed by Bug 388094.

For other values e.g. 61 minutes or 170 hours it still fails. I think the reason is the comparison in loadReminder() in <http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-dialog-utils.js#705>

170 hours is stored as -P7DT2H in the duration object. The comparison takes only the 2 hours into account but omits the 7 days -> 2 hours entries is selected in the menulist.

NOTE: This also happens for smaller reminder values e.g. 62 minutes.

62 minutes is stored as -PT1H2M in the duration object. In the dialog the 1 hour entry is selected, the minutes are omitted.

Comment 3

[Stefan Sitter [:ssitter]]

Attachment: rev0 - compare entire offset instead of single fields

As described in previous comment the problem is that the comparison worked on single fields of the duration. For example the check "duration.hours == 1" is true for all durations between 60 and 119 minutes.

This patch takes care off comparing the total duration using the calIDuration::inSeconds value.

In addition I moved the calculation of origin, duration and relation out of the for loop because it doesn't depend on the loop and to calculate/clone only once.

Comment 4

[Michael Büttner (no reviews TFN)]

Comment on attachment 280665 [details] [diff] [review]
rev0 - compare entire offset instead of single fields

Much better than the previous version. Thanks for the patch, Stefan. r=mickey.

Comment 5

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 6

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1.8pre) Gecko/20071002 Sunbird/0.7