Closed Bug 423320 Opened 14 years ago Closed 12 years ago

Calendar Unifinder: Setting filter updates tree incorrectly

Categories

(Calendar :: Calendar Frontend, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 508320

People

(Reporter: berend.cornelius09, Unassigned)

Details

Attachments

(1 file)

Steps to reproduce:
- Set the filter to the "Next 7 days"
- Create a Series of 5 events from today.
- Set The filter to today -> one event is correctly displayed
- Set the filter back to "Next 7 days"
- delete the event from tomorrow from the series
- Set the filter back to today -> no event is displayed which is wrong the event of today is still there.

I had a look at it and it seems to me that this is a problem of the rowcountChange in "setItems" in calendar-Unifinder.js in line 420.
Flags: blocking-calendar0.8?
Not a blocker, but we'll take a patch if one materializes.
Flags: wanted-calendar0.8+
Flags: blocking-calendar0.8?
Flags: blocking-calendar0.8-
Assignee: nobody → Berend.Cornelius
Attached patch patch v. #1Splinter Review
Attachment #311759 - Flags: review?(philipp)
Severity: critical → normal
Status: NEW → ASSIGNED
Comment on attachment 311759 [details] [diff] [review]
patch v. #1

>     removeItems: function uTV_removeItems(aItemArray) {
>         for each (var item in aItemArray) {
>             var row = this.getItemRow(item);
>             if (row > -1) {
>                 this.eventArray.splice(row, 1);
>                 if (this.tree) {
>                     this.tree.rowCountChanged(row, -1);
>                 }
>+                this.calculateIndexMap();
>             }
>         }
>-        this.calculateIndexMap();
>     },
This change will call calculateIndexMap each time an event is removed. Its better to do so after all items have been removed, otherwise runtime complexity is quite a bit higher.

What you can do is to only run the function if items have actually been removed, but I don't think its often the case that removeItems is being called with items that do not exist.

r=philipp for the first hunk.
Attachment #311759 - Flags: review?(philipp) → review+
Flags: wanted-calendar0.9+
Flags: wanted-calendar0.8+
Flags: blocking-calendar0.8-
As we discussed it seems necessary to call "calculateIndexMap" after each removing action because each removal action potentially also modifies the index map, which of course has a negative impact on the performance. Looking at the code in general I don't understand why all the action within and linked with "prepareCalendarUnifinder()" has to be taken place on load. Instead this could be called when th Unifinder is explicitly demanded either by a menu command or - when its visibility is persistent - on the modechange. We have cared so much about the performance of the unifinder. Therefor I think here it is worth while changing the onload behaviour towards a "onDemand" behaviour too.
Flags: blocking-calendar0.9?
Flags: blocking-calendar0.9? → blocking-calendar0.9-
Any updates on this?

r+ patch available, but no check-in - are you going to address comment #4 in this one, or do you want to file another bug on it?
Flags: wanted-calendar1.0+
Flags: wanted-calendar0.9+
Flags: blocking-calendar0.9-
Assignee: berend.cornelius09 → nobody
Status: ASSIGNED → NEW
This will be fixed by bug:
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 508320
You need to log in before you can comment on or make changes to this bug.