Closed Bug 511246 Opened 11 years ago Closed 11 years ago

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

Categories

(Calendar :: Calendar Views, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ingomu, Assigned: mmecca)

Details

(Whiteboard: [not needed beta][no l10n impact])

Attachments

(1 file, 1 obsolete file)

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).
Does this qawanted tag mean, that I should provide more info? If yes, which kind of info?
(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?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-calendar1.0?
Keywords: qawanted
OS: Linux → All
Hardware: Other → All
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.
Flags: blocking-calendar1.0? → blocking-calendar1.0+
Whiteboard: [not needed beta][no l10n impact]
Attached patch Proposed Patch (obsolete) — Splinter Review
Proposed patch: fixes deleteItemsFromCalendar method missing all day events in the column header.
Attachment #395497 - Flags: review?(philipp)
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.
Attachment #395497 - Flags: review?(philipp) → review+
Assignee: nobody → matthew.mecca
Status: NEW → ASSIGNED
Attached patch Patch v2Splinter Review
Attachment #395497 - Attachment is obsolete: true
Attachment #395914 - Flags: review?(philipp)
Comment on attachment 395914 [details] [diff] [review]
Patch v2

Matthew, you can carry forward Philipp's r+ if you just change what he suggested. :)
Attachment #395914 - Flags: review?(philipp) → review+
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/e0b82cec6f8b>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Great, thanks for the fix!
verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.1.5pre) Gecko/20091025 Calendar/1.0pre
Status: RESOLVED → VERIFIED
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.