373742 - week view changes after reload

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Matthias Schlett]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.2) Gecko/20070208 Iceweasel/2.0.0.2 (Debian-2.0.0.2+dfsg-1)
Build Identifier: Lightning Build 2007031203

When I reload a remote calendar, in the week view some day events are shown twice and some vanish. Moving to the next week changes only a part of the week days

Reproducible: Always

Steps to Reproduce:
1.open a week view with muliple all day events in several days
2.reload the remote calendar once or twice
3.click on the arrow to move to next week
Actual Results:  
After reload some all day events vanish and some shown twice.
After multiple arrow clicks sometimes a part of these wrong entries vanish and sometimes a part of the next week is shown.

Expected Results:  
the week view should stay unchanged after reload and clicking on the arrow should move to next week.

Comment 1

[Matthias Schlett]

Maybe the following Javscript console errors are related to this bug:
Error: dayHeaderBox.mItemBoxes[i] has no properties
Source File: chrome://calendar/content/calendar-multiday-view.xml
Line: 2522

Error: this.mDateColumns[0] has no properties
Source File: chrome://calendar/content/calendar-multiday-view.xml
Line: 2622

Error: gMsgFolderSelected has no properties
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 223

Comment 2

[Shinya Kuribayashi]

User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.3pre) Gecko/20070320 Calendar/0.5pre

Reproduced with Sunbird 0.5pre/Linux.

Matthias, I tested several versions and this isn't Lightning-dependent, but Linux-depenent.

0.3.1   | Sunbird    | Lightning
Linux   |   good     |     ?
Windows |      ?     |    good

0.5pre  | Sunbird    | Lightning
Linux   |   bad      |    bad
Windows |   good     |    good

Comment 3

[Matthias Schlett]

I tested it again with Lightning 2007032003 und  Thunderbird 20070116 under Windows with four all day events.
It has the same bad behavior as under Linux.

Comment 4

[Shinya Kuribayashi]

(In reply to comment #3)
> I tested it again with Lightning 2007032003 und  Thunderbird 20070116 under
> Windows with four all day events.

I still could not reproduce it with Lightning0.5pre/Windows even if with four all day events. Could we see your calendar file? If possible, pleast post that.

Comment 5

[Stefan Sitter [:ssitter]]

(In reply to comment #1)
> Maybe the following Javscript console errors are related to this bug:
> Error: dayHeaderBox.mItemBoxes[i] has no properties

This is the same error message as in Bug 374457 in connection with AutoReload and probably the same issue.

Comment 6

[Matthias Schlett]

Attachment: my calendar test file

Comment 7

[Shinya Kuribayashi]

Attachment: converted test calendar from Mattias

Thanks, finally reproduced with Lightning0.5pre/Windows. The point is that the all day event(s) has to be in the remote calendarm not 'On My Computer'. I didn't notice that, sorry.

Ok, update the matrix (re-tested):

0.3.1   | Sunbird    | Lightning
Linux   |   bad      |     ?
Windows |    ?       |    bad

0.5pre  | Sunbird    | Lightning
Linux   |   bad      |    bad
Windows |   bad      |    bad

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.4pre) Gecko/20070328 Calendar/0.5pre

After reloading, my workdays were empty, only a remote all day event on Friday was visible. For the next week, the same except the all day events were all visible. Going back to the first week showed the all day events correct, but the events during the days were still missing.

Comment 9

[Matthew (lilmatt) Willis]

This sounds pretty unusable.  Since we added the auto-reload stuff in this version, it should work.  

Comment 10

[glamken]

Confirmed with Sunbird 0.3.1 (german) on WinXP

Using PHP-generated remote ICS-Files.
After reload of remote ICS, some entries vanished.
After deaktivating remote calender some entries remain on week view.

This is different to other views, where everything is working perfectly.

Errorconsole shows:
Error: dayHeaderBox.mItemBoxes[i] has no properties
Sourcefile: chrome://calendar/content/calendar-multiday-view.xml
Line: 2370

Comment 11

[Sebastian Schwieger]

This bug has been confirmed on Linux and Windows, and not (yet?) on Mac. I'm changing the OS field to "All".

Comment 12

[Michael Büttner (no reviews TFN)]

I tried to find out what's going on here, this is what I found out so far... What's triggering the problem is the automatic refresh, which boils down to the following code:

        var cals = getCalendarManager().getCalendars({});
        for each (var cal in cals) {
            if (cal.canRefresh) {
                cal.refresh();
            }
        }

It depends from where this code gets executed. I removed the setUpRefreshTimer() function from calCalendarManager.js and executed a similar code from messenger-overlay-sidebar.js. Just to make sure that this gets executed from the UI thread. I had the impression that it has something to do with from which thread the above code gets triggered. Wrapping the above code in different types of timer stuff (nsITimer or setTimeout) both results in the exception getting triggered. Just executing the above 4 lines from inside e.g. toggleTasksInView() works just fine. It seems to be related to the fact that the refresh code gets triggered from a timer callback.

The exception that actually throws complains about the 'dayHeaderBox.mItemBoxes'-array. dayHeaderBox.mItemBoxes.length says that there are some elements in the array, but in the very next line accessing those elements throws the exception. The contents of the array have been vanished. It seems that the relayout() function gets called several times, but it's not re-entrant, something like this.

Currently, I don't exactly know what actually causes the problem. I'll keep trying to find out what's going on.

Comment 13

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

I finally got it. As it seems to be always the case, it is a unfortunate series of events that caused the problem. Accessing the mItemBoxes-array in [1] obviously referenced a non-existent object. I changed the for-loop into a while(length != 0)-loop and the problem disappeared. The question remained, what actually went wrong? deleteEvent() removes more than a single item (see [2]) if duplicate elements are stored in the array, which was the case here, which caused the bogus for-loop in [1] to access a non-existint object. Still no clue why all of this happens. Basically, what caused the trouble is the ICS-provider wrongly triggering observer-calls to addItem(), in [3]. In this case it is semantically wrong to tell the views that new items have been added. They were added to the internal memory calendar of the ICS-provider, nothing more. An elegant solution to this problem is to add the observer after the items have been added to the internal memory calendar. The patch contains this fix to the ICS-provider as well as the necessary fixes for the views to make them more error tolerant.

[1] http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-multiday-view.xml#2618
[2] http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-multiday-view.xml#1487
[3] http://lxr.mozilla.org/seamonkey/source/calendar/providers/ics/calICSCalendar.js#263

Comment 14

[Daniel Boelzle [:dbo]]

Comment on attachment 260593 [details] [diff] [review]
patch v1

>       <method name="addEvent">
>         <parameter name="aItem"/>
>         <body><![CDATA[
>+          // prevent same items being added
>+          for (var i in this.mItemBoxes) {
>+            if (this.mItemBoxes[i].occurrence.hasSameIds(aItem)) {
>+              return;
>+            }
>+          }
better use "if (this.mItemBoxes.some(<checkFunc>)) return;" here.

We have intensively discussed the inherent bug, i.e. the ics/memory provider notification upon initialization. An alternative fix would be to add an explicit initialization call to the memory calendar, but IMO this is not really necessary since the actual solution (postponing the observer being added on initialization) is well documented.

r=dbo

Comment 15

[Michael Büttner (no reviews TFN)]

Thanks for the hint regarding some(checkfunc), I changed the patch accordingly...

Checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 16

[Shinya Kuribayashi]

Verified, works fine.

0.5pre  | Sunbird    | Lightning
Linux   | 2007040603 | 2007040604
Windows | 2007040605 | 2007040603

Comment 17

[Martin Schröder [:mschroeder]]

Marking VERIFIED per comment#16.