511246 - All-day events don't get deleted from calendar-multiday-view onCalendarUnregistering

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Ingo Müller]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/4.2; Linux) KHTML/4.2.2 (like Gecko)
Build Identifier: Lightning 1.0pre nightly build from 2009-08-17

When a calendar gets deleted, it notifies its observers for an "onCalendarUnregistering" event. calendar-multiday-view correctly removes the non-all-day events of that calendar in its deleteItemsFromCalendar function. All-day events are not deleted and remain in the view until it is refreshed.

Reproducible: Always

Steps to Reproduce:
1. Create a calendar, open week view.
2. Create two events (on a day shown by the view): an all-day event and a normal one.
3. Delete the calendar.
4. Then advance one week and go back again (to refresh the view).
Actual Results:  
After 3, the normal event gets correctly removed from the view, whereas the all-day event remains there.
After 4, the view has correctly removed both events.

Expected Results:  
The view should remove both events right away (after 3).

Comment 1

[Ingo Müller]

Does this qawanted tag mean, that I should provide more info? If yes, which kind of info?

Comment 2

[Martin Schröder [:mschroeder]]

(In reply to comment #1)
> Does this qawanted tag mean, that I should provide more info? If yes, which
> kind of info?

Just wanted someone to triage it. ;) Confirmed using latest Lightning 1.0pre nightly on Windows XP. Ingo, thanks for your analysis of the issue. Maybe you want to contribute a fix for this bug?

Comment 3

[Philipp Kewisch [:Fallen][☁️📆]]

Strange. We want this for 1.0 final, but I guess I can live without it for the beta. Patches welcome though, it would obviously be nice to fix this.

Comment 4

[Matthew Mecca [:mmecca]]

Attachment: Proposed Patch

Proposed patch: fixes deleteItemsFromCalendar method missing all day events in the column header.

Comment 5

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 395497 [details] [diff] [review]
Proposed Patch

Great, thanks for the patch!


Regarding the loops for colEvents, we can do cool new things with js 1.8:

let colEvents = [ box.occurrence for each (box in col.header.mItemBoxes) ]
                .concat([ info.event for each (info in col.column.mEventInfos) ]);

>+
>+              for each (let event in colEvents) {
>+                  if (!deleteHash[event.hashId] && event.calendar.id == aCalendar.id) {
This might cause a strict warning. Better:  !(event.hashId in deleteHash)


r=philipp with this fixed, I'd appreciate a new patch I can directly checkin.

Comment 6

[Matthew Mecca [:mmecca]]

Attachment: Patch v2

Comment 7

[Martin Schröder [:mschroeder]]

Comment on attachment 395914 [details] [diff] [review]
Patch v2

Matthew, you can carry forward Philipp's r+ if you just change what he suggested. :)

Comment 8

[Martin Schröder [:mschroeder]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/e0b82cec6f8b>

-> FIXED

Comment 9

[Ingo Müller]

Great, thanks for the fix!

Comment 10

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.1.5pre) Gecko/20091025 Calendar/1.0pre