258956 - Recurrence does not show up when "All day event"

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Jesper Krogh]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040809 Firefox/0.9.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7) Gecko/20040809 Firefox/0.9.3

As can be seen in the screenshot, the "all-day-event" doesn't show up in the
list of events until next year. 


Reproducible: Always
Steps to Reproduce:
1.Create an all day event of the current day.
2.Select recurrence every year. 
 

Actual Results:  
The event does not show up on top of the list of events as an event of today,
the first occurrence in the list is a year from today. 

Expected Results:  
I'd expect it to show up today, as I can see it in the "day view"

Comment 1

[Jesper Krogh]

Attachment: Shot of the bug.

This is the same shot as the one that's linked to, showing the missing event in
the event list, with the event listed as it should in the "day view".

Comment 2

[gekacheka]

Confirmed.

The problem seems to be that getNextOrPreviousRecurrence is returning the next
recurrence even if now is during an occurence.  Desired behavior is to return
the current recurrence if during an occurence.

Function getNextOrPreviousRecurrence currently does something like the following
on recurring events:
  var start = event.getNextRecurrence(now)
  if (start == null)
     start = event.getPreviousRecurrence(now)
It only calls both if now is after the last recurrence.

A quick fix would be to rewrite getNextOrPreviousRecurrence to 
  var dur = event.end - event.start;
  var start = event.getPreviousRecurrence(now);
  if (start == null || start + dur < now)
    start = event.getNextRecurrence(now);
However, this calls both getPreviousRecurrence and getNextRecurrence for all
future recurrences.  getNextOrPreviousRecurrence is called frequently (e.g.,
sorting the AllEvents list), so it would be better to avoid calling them both.

A better approach might be to provide provide a version of
event.getNextRecurrence, event.getCurrentOrNextRecurrence, that returns the
current ocurrence if time is during one (check if times is before the end date
rather than the start date).
http://lxr.mozilla.org/mozilla/source/calendar/libxpical/oeICalEventImpl.cpp#1126
Then getNextOrPreviousRecurrence would not need to be changed except to call
event.getCurrentOrNextRecurrence instead of event.getNextRecurrence.




 

Comment 3

[Chris Sanner]

Attachment: Recurrance Bug screenshot (not for this bug)

Comment 4

[Chris Sanner]

I confirm this bug - my oncall status is an "all day event" stretching from
monday of one week to monday of the next.  When I set this even to recurr, only
the first two days show up.  In fact, only one day would show up, except for
another bug that, when creating new all-day events, causes two days to be marked
instead of only one.

Comment 5

[Chris Sanner]

(In reply to comment #4)
> I confirm this bug - my oncall status is an "all day event" stretching from
> monday of one week to monday of the next.  When I set this even to recurr, only
> the first two days show up.  In fact, only one day would show up, except for
> another bug that, when creating new all-day events, causes two days to be marked
> instead of only one.

er...shoot.  wrong bug.  needed to read ahead further...

Comment 6

[gekacheka]

Attachment: calendarEvent.js, unifinder.js: getNextOrPrevious --> getCurrentNextOrPrevious

Can use calendarEvent.getNextRecurrence to return current recurrence if one
exists by using a start probeDate that is (now - duration) instead of now. 
(So, similar to before, it only calls both getNextRecurrence and
getPreviousRecurrence if probe is past last recurrence date, avoiding the
performance issue.)

calendarEvent.js:
  rename getNextOrPreviousRecurrence to getCurrentNextOrPreviousRecurrence
  modify to use probeDate of (now - duration) as described above.

Modify calls to getNextOrPreviousRecurrence 
  to getCurrentNextOrPreviousRecurrence.
  in calendarEvent.js and unifinder.js

Comment 7

[gekacheka]

Comment on attachment 159295 [details] [diff] [review]
calendarEvent.js, unifinder.js:  getNextOrPrevious --> getCurrentNextOrPrevious

tested on SB 0.2 (moz1.8), TB 0.8 (moz1.7).

Create yearly all-day event that falls on today.  Without patch, event listing
shows it at next year's date.  With patch, shows it today even though it
already started (at midnight).

Comment 8

[gekacheka]

(patch also removes a duplicate copy of getNextOrPreviousRecurrence in unifinder.js)

Comment 9

[Mostafa Hosseini]

This instance of getNextOrPreviousRecurrence has been left out from unifinder.js

@@ -724,7 +724,7 @@
          gCalendarWindow.EventSelection.setArrayToSelection( ArrayOfEvents );
       
          /*start date is either the next or last occurence, or the start date
of the event */
-         var eventStartDate = getNextOrPreviousRecurrence( SelectedEvent );
+         var eventStartDate = getCurrentNextOrPreviousRecurrence( SelectedEvent );
             
          /* you need this in case the current day is not visible. */
          gCalendarWindow.currentView.goToDay( eventStartDate, true);

Comment 10

[Mostafa Hosseini]

Checked in along with change in comment #9.
Thanks

Comment 11

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: General. Be afraid for your sanity!