Closed Bug 680620 Opened 10 years ago Closed 2 years ago

Distant event end date hangs in calendar views

Categories

(Calendar :: Calendar Frontend, defect)

Lightning 1.0b5
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hal, Assigned: Fallen)

References

(Regressed 1 open bug)

Details

(Keywords: hang, perf, testcase)

Attachments

(2 files, 4 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0
Build ID: 20110811165603

Steps to reproduce:

Inadvertently entered an spurious extra digit in the calendar end date (I think). 


Actual results:

Next time I loaded Thunderbird it first didn't seem to load and I finally noticed Thunderbird was running but just increasing in stored file size..


Expected results:

The program should have noted or trapped the bad date causing the calendar to try to increase to an impossible size. I think that was what was going on. Only got Thunderbird back mon line by blowing away the calendar data and loading a new Thunderbird install.
Severity: normal → critical
Keywords: hang, perf
Product: Thunderbird → Calendar
QA Contact: general → general
Version: 6 → Lightning 1.0b5
Do you still have the calendar data? Maybe you can send it to me and I'll take a look at what is causing the issue.
I can confirm this issue, but I think something similar has already been reported (but I can't find it).

Step to reproduce:
1. on a new profile (it will become unusable) create a new event with an end date far into the future e.g. with a five digits year (by adding a digit to the date proposed in the datepicker);
2. save and close the dialog;
3. Lightning hangs and after some time the dialog "Warning: unresponsive script" appears;
4. stop the script (you can try to continue, but it seems an ever ending process). Lightning works as usual. The event doesn't appear in the view but you can see it in the unifinder with the five digits year as end date (if you try to delete, Thunderbird hangs). Exporting the ics calendar, it results an event without the end date:

BEGIN:VEVENT
CREATED:20110821T065614Z
LAST-MODIFIED:20110821T065621Z
DTSTAMP:20110821T065621Z
UID:d52837c2-e471-442a-b6a4-74ac4f9de1f3
SUMMARY:New Event
DTSTART;TZID=Europe/Paris:20110824T090000
END:VEVENT

5. close and restart Thunderbird, look the task manager: Thunderbird process' memory increases continuously but Thunderbird's window doesn't appear. At the end you have to kill the process and run Thunderbird on another profile.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Creating an event with an end date of 2050, for example, also takes a long time to appear...

The code of the views seem to cause this problem.

For the month view, here are the recorded times (without the patch) :

5. *** calendar-month-view.xml relayout - duration:'141 ms'
8. *** calendar-month-view.xml relayout - duration:'23 ms'
10. *** minimonth.xml setBusyDaysForOccurrence - duration:'3406 ms'
11. *** minimonth.xml setBusyDaysForOccurrence - duration:'3419 ms'
12. *** minimonth.xml setBusyDaysForOccurrence - duration:'3411 ms'
13. *** minimonth.xml setBusyDaysForOccurrence - duration:'3439 ms'
16. *** calendar-month-view.xml findDayBoxesForItem - duration:'7805 ms'
20. *** calendar-month-view.xml findDayBoxesForItem - duration:'7835 ms'
24. *** calendar-month-view.xml findDayBoxesForItem - duration:'3322 ms'
27. *** calendar-month-view.xml relayout - duration:'13 ms'
30. *** calendar-month-view.xml relayout - duration:'6 ms'
32. *** minimonth.xml setBusyDaysForOccurrence - duration:'3389 ms'
35. *** calendar-month-view.xml findDayBoxesForItem - duration:'7828 ms' 

In the function 'findDayBoxesForItem', we can see that we call 'findDayBoxForDate' for all dates of the event.
In the patch, I limit the end date based this.endDate.
But there is a case where 'this.endDate' is not defined. setDateRange is not called before calling findDayBoxesForItem.

With the patch, we get:

4. *** minimonth.xml setBusyDaysForOccurrence - duration:'3 ms'
6. *** minimonth.xml setBusyDaysForOccurrence - duration:'3 ms'
8. *** minimonth.xml setBusyDaysForOccurrence - duration:'2 ms'
10. *** minimonth.xml setBusyDaysForOccurrence - duration:'2 ms'
12. *** calendar-multiday-view.xml findColumnsForItem - duration:'0 ms'
16. *** calendar-month-view.xml findDayBoxesForItem - duration:'3311 ms'
20. *** calendar-month-view.xml findDayBoxesForItem - duration:'3304 ms'
22. *** calendar-multiday-view.xml findColumnsForItem - duration:'1 ms'
24. *** minimonth.xml setBusyDaysForOccurrence - duration:'2 ms'
26. *** calendar-multiday-view.xml findColumnsForItem - duration:'0 ms'

It is a case where findDayBoxesForItem takes time. setDateRange is not called in this case.
This causes the patch is not yet complete. 
Upon restart, the application will continue to loop because setDateRange before calling findDayBoxesForItem (calendar-month-view.xml).

In the week view, it seems that findDayBoxesForItem (calendar-month-view.xml) is also called when starting the application.
(traces with patch)
4. *** minimonth.xml setBusyDaysForOccurrence - duree:'3 ms'
6. *** minimonth.xml setBusyDaysForOccurrence - duree:'3 ms'
8. *** minimonth.xml setBusyDaysForOccurrence - duree:'2 ms'
10. *** minimonth.xml setBusyDaysForOccurrence - duree:'2 ms'
12. *** calendar-multiday-view.xml findColumnsForItem - duree:'0 ms'
16. *** calendar-month-view.xml findDayBoxesForItem - duree:'3311 ms'
20. *** calendar-month-view.xml findDayBoxesForItem - duree:'3304 ms'
22. *** calendar-multiday-view.xml findColumnsForItem - duree:'1 ms'
24. *** minimonth.xml setBusyDaysForOccurrence - duree:'2 ms'
26. *** calendar-multiday-view.xml findColumnsForItem - duree:'0 ms'
Attachment #560545 - Flags: review?(philipp)
Ooh nice, perf patches in the views! From a first glance the patch looks good, I'll give it a test run soon.
Attached patch Patch v2 (obsolete) — Splinter Review
Similar problems occur with an event whose start date is far in the past. The display is slow with an event whose start date is far in the past.
Patch v2 handles these 2 case (past / future).

At startup, the patch sets the date range (setDateRange) by calling 'goToDay' for all views.
Attachment #567447 - Flags: review?(philipp)
Comment on attachment 560545 [details] [diff] [review]
Patch that prevents looping when creating an event with a date far into the future

Has newer patch attached.
Attachment #560545 - Attachment is obsolete: true
Attachment #560545 - Flags: review?(philipp)
Assignee: nobody → philippe.martinak
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: x86 → All
Duplicate of this bug: 710596
Attachment #567447 - Attachment is obsolete: true
Attachment #567447 - Flags: review?(philipp)
Attachment #584551 - Flags: review?(philipp)
Attached patch Fix - v3 (obsolete) — Splinter Review
Hi Philippe, sorry for the delay. I've tested your patch and have a suggestion for improvement: When Lightning starts, "today" is selected in the currently visible view. Instead of setting the selectedDay through all views on load, the views could just make sure that selectedDay never returns null but instead sets now() if selectedDay is not set. I have modified the patch for this, let me know what you think and if it still does what is expected.
Attachment #584551 - Attachment is obsolete: true
Attachment #584551 - Flags: review?(philipp)
Attachment #589140 - Flags: review+
Attachment #589140 - Flags: feedback?(philippe.martinak)
Attached patch Fix - v4Splinter Review
Oops, forgot to refresh the patch
Attachment #589140 - Attachment is obsolete: true
Attachment #589140 - Flags: feedback?(philippe.martinak)
Attachment #589142 - Flags: review+
Attachment #589142 - Flags: feedback?(philippe.martinak)
(In reply to Philipp Kewisch [:Fallen] from comment #10)
> Created attachment 589142 [details] [diff] [review]
> Fix - v4
> 
> Oops, forgot to refresh the patch

Hi Philipp, 

i just made some tests and i've noticed that events with end date far in future are not displayed in the last cell of the view.
ex : 
add event 1/17/2012-2/17/2012 -> the last cell is empty in the view

we need to modify:
finishDate=this.endDate.clone();
by:
finishDate=this.endDate.clone();
finishDate.day+=1;

in calendar-month-view.xml and calendar-multiday-view.xml files
Comment on attachment 589142 [details] [diff] [review]
Fix - v4

Feedback of pmartinak in comment#11 reads like feedback-.
Attachment #589142 - Flags: feedback?(philippe.martinak) → feedback-
Reassigning to Philipp as he created the last version of the patch.
Assignee: philippe.martinak → philipp
Component: General → Calendar Views
QA Contact: general → views
related to bug 350463?
Summary: Bug in Calendar Results in an Infinite (or near infinite) Loop in Thunderbird → Distant event end date hangs in calendar views
Duplicate of this bug: 796853
Note also there is a testcase on bug 796853 which has a similar effect. Fixing this bug should make sure that one is also covered.
sanity check?
Keywords: testcase
Whiteboard: [patchlove]
Duplicate of this bug: 767780
Duplicate of this bug: 1510401

This is surely all it takes to stop looking for date boxes outside the current range.

Attachment #9056294 - Flags: review?(philipp)
Attachment #9056294 - Flags: review?(philipp) → review+

More beta fodder?

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/8eb6c4b7ccd4
Optimise display of events that extend outside the current calendar view; r=Fallen

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 7.0
Comment on attachment 9056294 [details] [diff] [review]
680620-long-event-1.diff

Let's get this in beta. Then in ESR.
Attachment #9056294 - Flags: approval-calendar-beta?(philipp)
Attachment #9056294 - Flags: approval-calendar-beta?(philipp) → approval-calendar-beta+

no regressions so far https://mzl.la/2J7Po0T and good enough for next esr?

Whiteboard: [patchlove]
Comment on attachment 9056294 [details] [diff] [review]
680620-long-event-1.diff

> good enough for next esr?

Seems reasonable. I didn't realise it had made it to beta already.
Attachment #9056294 - Flags: approval-calendar-esr?(philipp)
Attachment #9056294 - Flags: approval-calendar-esr?(philipp) → approval-calendar-esr+
You need to log in before you can comment on or make changes to this bug.