Closed Bug 419860 Opened 12 years ago Closed 12 years ago

Unifinder 'All Future Events' shows past events

Categories

(Calendar :: Calendar Views, defect, major)

defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: Hb, Assigned: sebo.moz)

References

()

Details

(Keywords: regression)

Attachments

(2 files)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13pre) Gecko/20080226 Calendar/0.8pre

Past events are shown in unifinder even if the event filter is set to "All Future Events". It is no difference to the output of the filter "All Events" if the search field is empty.

1. Create two events named "jan" in January 2008 and "march" in March.
2. Set "All Events" leaving the search field empty. You see both events.
3. Change to "All Future Events". You still see both events, under Sunbird 0.7 you would have seen "march" only.

Additional info:

4. Reload. You still see both.
5. Type "a" in the search field. Now the filter comes to effect and you see only "march".
Flags: blocking-calendar0.8?
taking over...
Assignee: nobody → Berend.Cornelius
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13pre) Gecko/2008022621 Calendar/0.8pre

Works fine using storage provider. Error confirmed using ics provider.

Could someone test if other providers (caldav, wcap, gdata) are working or failing too?
I checked this:

- local storage calendar -> test scenario works
- WCAP calendar -> test scenario works
- gdata calendar - test scenario works
- webdav calendar -> test scenario works NOT
 
OS: Windows XP → All
Hardware: PC → All
once more:

- calDAV calendar -> test scenario works NOT, too.

The calDav server was http://rscds.zathras.lss.wisc.edu/caldav.php/mozilla/home/ (QA_Test_Server page) 
If we can get a fix within the next 5-7 days, then fine. Otherwise while this is ugly, the app is still perfectly usable with this bug, so we may have to re-evaluate this if no fix appears.
Status: NEW → ASSIGNED
Flags: blocking-calendar0.8? → blocking-calendar0.8+
This unit test emulates the unifinder conditions for the getItems call:
startRange: someDate..
endRage = null;
filter = cal.ITEM_FILTER_TYPE_EVENT;

Indeed memoryCalendar behaves faulty and returns test event number 5 which ends before startRange.
Attached patch patchSplinter Review
This fixes checkIfInRange.
Assignee: Berend.Cornelius → sebo.moz
Attachment #306351 - Flags: review?
Attachment #306351 - Flags: review? → review?(Berend.Cornelius)
Using ics calendar I get the following regression range:
    Works in Sunbird 0.8pre (2008-02-01-20)
    Fails in Sunbird 0.8pre (2008-02-02-14)

Checkins during regression range: http://tinyurl.com/3ar7yn
                                  Bug 408652, Bug 321010, Bug 413715
Sebo, does your patch fix the issue? It doesn't seem to do so for me, creating an event that ends at 11:37 while it is 11:38 still shows in the unifinder.
(In reply to comment #9)
> Sebo, does your patch fix the issue? It doesn't seem to do so for me, creating
> an event that ends at 11:37 while it is 11:38 still shows in the unifinder.
> 
The unifinder takes the start of the day as the rangeStart. An event from yesterday should not be shown.
As sebo already mentioned the calendar-Unifinder comparison-range is the start of the day. It probably makes mores sense to take now() as the basis for this, but I find this is not the topic for this issue. Looking at the code in "CheckIfInRange" I cannot see any reason why your modification should not improve the situation. Yet my concern is that this is a very central function called from all thinkable contexts and I do not really want to bet on it that sebo's modification does not change something to the worse in another context. Therefor I just want to put up for discussion if we really want to take this for 0.8. Andreas voted to put it from the blocker list.
Comment on attachment 306351 [details] [diff] [review]
patch

r=berend.
Attachment #306351 - Flags: review?(Berend.Cornelius) → review+
Not putting these brackets checkIfInRange is just wrong since all items are returned if rangeEnd is null. I would vote to take this for 0.8 as it simply removes an obvious bug.
Yes Sebastian is quite right. The current code is just so wrong that it should be removed in any case. I was just cautious, because we are so short before a release and this is a very central function.
Does the patch solve the issue? I'm not sure if that part of the code is called from the unifinder. I placed a debugger; statement just before the clause and it doesn't shows up in venkman.

The unifinder should work on daily base so the "future" should start at 00:00 of the current day.
I have had the same problem. In my debugger calUtils.js occured twice and I had to open the second "calUtils.js" to debug into it. But then I could clearly debug into it.
Argh. I first patched the file under /chrome/calendar/content/ but called is only the one under /js/. After patching this file the issue is solved.
patch checked in on trunk and MOZILLA_1_8_BRANCH.

=> FIXED
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Checked with nightly build 2008031219 -> task is fixed and verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.