While on the plane this past weekend, I wasn't able to work on 0.1 blockers for various reasons, so I poked at a few other things.
More complete description of the bug: if you add or delete a calendar, it's still listed in the calendar pane until you manually go to another tab and then come back, forcing a repaint.
Dmose, is this related to Bug 305578? Not the same problem but maybe the same cause?
Stephan pointed out there there's a really bad failure mode to this bug: since there's not currently any confirmation for deleting a calendar, someone is reasonably likely to hit delete twice since it appears as though nothing happens, thus deleting two calendars. Adding this back to the blocking list for now. Alterately, fixing bug 298373 before 0.1 would allow us to drop this bug from the blocker list.
(In reply to comment #3) > Dmose, is this related to Bug 305578? Not the same problem but maybe the same > cause? > I kinda suspect not, but I'm not really sure. There are several bugs along those lines, however, and some of them are probably related.
Created attachment 210403 [details] [diff] [review] patch, v2 This patch removes onCalendarUnregistering, since it's insufficent for our needs here, and replaces it with onCalendarUnregistered. At some point in the future we can re-add onCalendarUnregistering, should it be necessary for something else. As far as the XXX comments in the patch, addressing them will, at best, give us a slight performance improvement. I propose to file another bug to look into that down the road.
Comment on attachment 210403 [details] [diff] [review] patch, v2 I don't really understand how this could work: this.mUnregisterCalendar.step(); this.mUnregisterCalendar.reset(); // delete prefs for the calendar too pp = this.mDeletePrefs.params; pp.calendar = calendarID; this.mDeletePrefs.step(); this.mDeletePrefs.reset(); delete this.mCache[calendarID]; + + this.notifyObservers("onCalendarUnregistered", [calendar]); and then when we notify the observer, that calls removeCalendarFromTree(aCalendar) which does + var changedRow = getCalendars().indexOf(aCalendar); + boxobj.rowCountChanged(changedRow, -1); So we remove the calendar from the database, and then try to find its index in the list of calendars? Either I'm not understanding this codeflow correctly, or something is wrong here.
Created attachment 210501 [details] [diff] [review] patch, v3 Good catch, Joey. I was misunderstanding how my own patch was working. The real problem here is the lack of identity of "interface pointers" in JS. Here's a new hackaround. I'll post two more bugs about that (one for fixing in XPConnect, and one for tweaking the calendar interfaces to workaround XPConnect so that we continue to play nice in 1.5). Then I'll file another bug to undo this hackaround once the calendar interfaces bug workaround lands.
Comment on attachment 210501 [details] [diff] [review] patch, v3 I suppose this is a pretty minimal perf hit. Please cite the bug number of the bug you filed about interface pointers in the comment though. r=jminta with that.
Attachment #210501 - Flags: first-review?(jminta) → first-review+
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
For some reason, this seems to have regressed. However, since we now prompt for deletion, I dont think this regression needs to block 0.1. I noticed this before I landed bug 328597 just now, so that is not the cause.
No longer blocks: 298366
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
A bunch of different people have complained about this since 0.3RC1, if we can get a fix in time that's suitably low-risk, we should consider taking it.
Whiteboard: [patch needed]
Target Milestone: --- → Lightning 0.3
I suspect the combination of the patches that landed in bug 328597 and bug 296202 caused this to regress. In particular, we now both unregister and delete calendars in Lightning, and the Lightning observers for each of those actions both try to remove the calendar from the tree. That said, I'm not able to reproduce this bug in my trunk build at the moment, though I do see it in the RC1 branch build. So there may be more to this yet.
OS: Windows XP → All
Hardware: PC → All
The regression is better documented in bug 352667. Re-closing here.
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago → 12 years ago
Resolution: --- → FIXED
Target Milestone: Lightning 0.3 → ---
You need to log in before you can comment on or make changes to this bug.