Closed Bug 507105 Opened 16 years ago Closed 16 years ago

Task list doesn't update correctly when adding/removing calendars.

Categories

(Calendar :: Tasks, defect)

Product:
Calendar
Component:
Tasks
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: Fallen, Assigned: Fallen)

References

Details

(Keywords: regression, Whiteboard: [needed beta][no l10n impact])

Attachments

(2 files)

[checked in] the fix
6.47 KB, patch
Fallen
: review+
Details | Diff | Splinter Review
Fix - v2
1.64 KB, patch
mschroeder
: review+
Details | Diff | Splinter Review
bug 500718 improved handling when unchecking calendars. This caused regression bug 506322, which had a patch that only removes the most obvious error. Wolfgang, could you find out why the task list is not refreshing correctly?
Flags: blocking-calendar1.0+
Oops, the second mentioned bug should be bug 506608.
Blocks: 500718
Whiteboard: [needed beta][no l10n impact]
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2pre) Gecko/20090729 Calendar/1.0pre (BuildID: 20090729050908) Steps to Reproduce: Setup multiple calendars with multiple tasks. * Add or Switch On a calendar -> Actual: task list shows only tasks from the new added calendar -> Expected: task list shows tasks from all enabled calendars * Remove or Switch Off a calendar -> Actual: task list did not update -> Expected: task list shows only the tasks from the remaining calendars
Keywords: regression
Wolfgang, any idea what is going on here?
(In reply to comment #3) > Wolfgang, any idea what is going on here? Yes. I am currently working on the same issue in 0.9 (Inverse ed.). I think the problem is double. There is one bug in calendar-item-editing which tries to set the endDate in checkAndSendItipItem and another bug in the handling of occurrences. I will probably produce a valid patch tomorrow. Note that it all could be a big patch along with: https://bugzilla.mozilla.org/show_bug.cgi?id=502936#c15 , if someone would just review that code I posted 4 weeks ago...
I observed this issue in Sunbird therefore it can't be caused by invitation handling. It is a regression from the calendar handling changes in bug 500718.
(In reply to comment #5) > I observed this issue in Sunbird therefore it can't be caused by invitation > handling. It is a regression from the calendar handling changes in bug 500718. I have taken a look to Lightning 1.0b. There are so many changes that I made both for this fix and 502936 that I will wait that my patch has been reviewed before doing anything else. It would not make sense to not apply both at the same time and I don't have much time to spend on this otherwise.
Tasks would not be removed in onCalendarRemoved because of the problem fixed at line 158: - var tasks = Array.splice(this.mTaskArray); + this.mTreeView.treebox.beginUpdateBatch(); + var tasks = this.mTaskArray.concat([]); Tasks would disappear from other calendar during onCalendarAdded because the array of old tasks would be completely wiped. The fix is to keep all the tasks of the other calendars and add a "aCompleteRefresh" parameter for the "refresh" method.
Attachment #395132 - Flags: review?
Attachment #395132 - Flags: review? → review?(philipp)
Comment on attachment 395132 [details] [diff] [review] [checked in] the fix There were a few places to change var to let, a few places to prefix functions with cal. and a few indentation problems. I took care of them all locally. r+ codewise, I'll give this a test later on and then check in. > onCalendarAdded: function tTO_onCalendarAdded(aCalendar) { >- if (!this.mInBatch && !aCalendar.getProperty("disabled")) { Just out of curiosity, why is it needed to remove the batch handling here? > <method name="refreshFromCalendar"> >+ for each (let task in this.mTaskArray) { >+ if (task.calendar.id != aCalendar.id) { >+ newArray.push(task); >+ } >+ } newArray = [ task for each (task in this.mTaskArray) if (task.calendar.id != aCalendar.id) ]; > <method name="refresh"> > <parameter name="aFilter"/> > <body><![CDATA[ >- this.refreshFromCalendar(getCompositeCalendar(), aFilter); >+ this.mTreeView.treebox.beginUpdateBatch(); >+ this.refreshFromCalendar(getCompositeCalendar(), aFilter, true); >+ this.mTreeView.treebox.endUpdateBatch(); This won't really work as expected, since refreshFromCalendar is asynchronous. In the worst case, the batch process will begin, the network requests will be started, the batch process will end, and then the onOperationComplete will be called. Lets move the tree batch stuff to a different bug, unless you want to take care here. >- var tasks = Array.splice(this.mTaskArray); >+ this.mTreeView.treebox.beginUpdateBatch(); >+ var tasks = this.mTaskArray.concat([]); I'll leave this batch stuff in though, nothing async here.
Attachment #395132 - Flags: review?(philipp) → review+
(In reply to comment #8) > > <method name="refresh"> > > <parameter name="aFilter"/> > > <body><![CDATA[ > >- this.refreshFromCalendar(getCompositeCalendar(), aFilter); > >+ this.mTreeView.treebox.beginUpdateBatch(); > >+ this.refreshFromCalendar(getCompositeCalendar(), aFilter, true); > >+ this.mTreeView.treebox.endUpdateBatch(); > This won't really work as expected, since refreshFromCalendar is asynchronous. > In the worst case, the batch process will begin, the network requests will be > started, the batch process will end, and then the onOperationComplete will be > called. Lets move the tree batch stuff to a different bug, unless you want to > take care here. The way the code is now, it is also not needed. when savedThis.onOperationComplete() is called, the tree view is reset, so no batch processing needs to happen anyway.
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/f28f880c5300> -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
> * Switch Off a calendar > -> Actual: task list did not update > -> Expected: task list shows only the tasks from the remaining calendars I can still reproduce this issue using Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.4pre) Gecko/20090826 Calendar/1.0pre.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Wolfgang, could you take another look?
Stefan, if with "switch off" you mean disable, I cannot reproduce. It seems to work for me now?
With "switch off" I mean: Open the calendar properties dialog and remove the check-mark next to "Switch this calendar on". I can still reproduce using Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.4pre) Gecko/20090902 Calendar/1.0pre.
Duh, I was looking in the unifinder. I can also confirm.
Attached patch Fix - v2 — — Splinter Review
This should take care
Assignee: WSourdeau → philipp
Status: REOPENED → ASSIGNED
Attachment #398842 - Flags: review?(mschroeder)
Attachment #395132 - Attachment description: the fix → [checked in] the fix
Whiteboard: [needed beta][no l10n impact] → [needed beta][no l10n impact][needs review]
Attachment #398842 - Flags: review?(mschroeder) → review+
Comment on attachment 398842 [details] [diff] [review] Fix - v2 r=mschroeder. Works as expected!
Whiteboard: [needed beta][no l10n impact][needs review] → [needed beta][no l10n impact]
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/f4f457ee162b> -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: