Closed Bug 373742 Opened 13 years ago Closed 13 years ago

week view changes after reload

Categories

(Calendar :: Calendar Views, defect, major)

x86
All
defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: m.schlett, Assigned: michael.buettner)

References

Details

(Whiteboard: [no l10n impact])

Attachments

(3 files)

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.
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
Severity: normal → major
Version: unspecified → Trunk
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
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.
(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.
(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.
Attached file my calendar test file
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
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Lightning Only → Calendar Views
Flags: blocking-calendar0.5?
QA Contact: lightning → views
This sounds pretty unusable.  Since we added the auto-reload stuff in this version, it should work.  
Flags: blocking-calendar0.5? → blocking-calendar0.5+
Whiteboard: [no l10n impact] [needs patch]
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
This bug has been confirmed on Linux and Windows, and not (yet?) on Mac. I'm changing the OS field to "All".
OS: Linux → All
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.
Attached patch patch v1Splinter Review
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
Assignee: nobody → michael.buettner
Status: NEW → ASSIGNED
Attachment #260593 - Flags: first-review?(daniel.boelzle)
Whiteboard: [no l10n impact] [needs patch] → [no l10n impact] [needs review]
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
Attachment #260593 - Flags: first-review?(daniel.boelzle) → first-review+
Thanks for the hint regarding some(checkfunc), I changed the patch accordingly...

Checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact] [needs review] → [no l10n impact]
Verified, works fine.

0.5pre  | Sunbird    | Lightning
Linux   | 2007040603 | 2007040604
Windows | 2007040605 | 2007040603
Marking VERIFIED per comment#16.
Status: RESOLVED → VERIFIED
Depends on: 382840
Flags: blocking-calendar0.5+
You need to log in before you can comment on or make changes to this bug.