459803 - Searching with unifinder causes error output and doesn't work

Status: VERIFIED FIXED

(Calendar :: General, defect)

Description

[Andreas Treumann]

STEPS TO REPRODUCE:
===================

- open the unifinder
- insert a character in the 'contain' text box
- check the error console

RESULT:
=======

- error output:

Error: fixAlldayDates is not defined
Source File: chrome://calendar/content/calFilter.js
Line: 313

- the filtering doesn't work  

EXPECTED RESULT:
================

- no error output

REPRODUCIBLE:
=============

- always

Comment 1

[Fred Jendrzejewski]

Attachment: Removed the lines causing the error

I think they aren't needed anymore, why should we introduce a filter looking for date in a text filter, so I removed it.

Comment 2

[Stefan Sitter [:ssitter]]

The function was originally added with Bug Bug 306157 to correctly handle all day events in the unifinder results. Please ensure that you don't regressed this.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/calendar/base/content/calendar-unifinder.js&mark=&rev=1.25&mark=73-75#72

Comment 3

[Fred Jendrzejewski]

I think that I took care of it, but I was never sure. IMO this function does what checkIfInRange does. Passing the start and the end date of the filter together with the item should to this function work.This is exactly what isItemWithinRange does. And isItemWithinRange is always applied when I call isItemInFilters.

And everywhere else in the code where the fixAlldayDates function was I replaced it with isItemInFilters to be sure, but actually I am still not sure if I need it always.

Comment 4

[Fred Jendrzejewski]

Comment on attachment 343020 [details] [diff] [review]
Removed the lines causing the error

Change the review request. Philipp introduced this function, so he might be the one who knows best if we still need it.

Comment 5

[Stefan Sitter [:ssitter]]

(In reply to comment #2)
I did a short test with storage and ics provider and so far it seems to work correctly after removing the function call.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 343020 [details] [diff] [review]
Removed the lines causing the error

Andreas tested this for me with numerous providers and it seems to no longer be required.

r=philipp

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

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

-> FIXED

Comment 8

[Andreas Treumann]

Checked in lightning build 20081021041724 and Sunbird 20081021 -> VERIFIED.