GNOME Bugzilla – Bug 528902
time zone handling insufficient
Last modified: 2014-03-25 16:55:44 UTC
Please describe the problem: Evolution's time zone handling fails to handle the fact that time zone definitions can change. This causes events to be displayed at the wrong time (one hour too early/too late). There are two related problems: Importing a new meeting invitation into a calendar which already contains an old time zone definition ignores the new time zone definition. The new meeting is then displayed using the old definition. The biggest grief has been caused (and still is caused) by the change of the US summer saving rules that became effective in 2007. Many users still incorrectly use the old rules because they imported Outlook meeting invitations prior to that change. The other problem is that there is no mechanism to update the time zone rules used by existing meetings if their time zone changes. Evolution should be able to handle that even if no new, updated meeting invitation is sent. Steps to reproduce: 1. create a new local calendar 2. set Evolution temporarily to "UTC" 3. import the attached 2006.ics 4. import the attached 2008.ics 5. look at March 25th 2008 6. look at March 21st 2006 Actual results: Both "One Time Meeting" and "Weekly Phone Call" are displayed as occurring at 13:30 in 2008, which is wrong. In 2006 "Weekly Phone Call" is displayed as occurring at 13:30, which is correct. Expected results: The correct start time would be 12:30 in 2007 and 2008 because the switch between normal and daylight saving time changed in 2007. In 2006 the old rules must be applied. Does this happen every time? Yes. Other information: I am going to suggest a solution and together with a patch to implement it.
Created attachment 109530 [details] recurring meeting starting in 2006, using 2006 US daylight saving rules
Created attachment 109531 [details] one-time event on March 25th 2008, using the same time zone ID as the 2006 event, but with updated time zone definition
There are three different problems: 1. time zone definitions should be attached to events, not calendars: that way conflicting definitions can both be preserved 2. instead of using custom time zone definitions sent together with a meeting, the system time zone definitions should be used whenever possible: the custom definitions included in meeting invitations are often incomplete (= only contain the current rules) and don't get updated unless the meeting invitation is sent again 3. when using system time zones the historic rules must be taken into account; currently Evolution only uses the latest rules and thus may display historic events incorrectly (note that this is my understanding of the situation; I might have misunderstood something) Implementing 1 as is would require considerable conceptual changes. My proposal is to detect the time zone conflicts when importing a new time zone and save it under a different TZID instead. This can be combined nicely with mapping of TZIDs to system TZIDs. The exact location where that time zone checking has to be done remains to be decided: Evolution just sends a VCALENDAR to the calendar backend via e_cal_backend_sync_receive_objects() and the individual backends then implement the importing. One could patch e_cal_backend_sync_receive_objects(), but that would come with a runtime overhead and prevents different solutions to the problem inside different backends. I suggest to put the time zone checking into an utility function "e_cal_check_timezones()" which can be used by those backends which work with icalcomponents internally. I have tested this approach with the file backend and will send a patch. The same utility function is also needed by SyncEvolution and other clients which import VCALENDAR events themselves: SyncEvolution first parses the VCALENDAR, adds time zone definitions via e_cal_add_timezone(), and then the event via e_cal_create_object() or e_cal_modify_object(). This approach is necessary because more control over event manipulation as well as the new UID of the event are required. SyncEvolution 0.8 alpha 1 already uses the function. That way events imported by SyncEvolution into Evolution (even older releases which are not going to be updated) works correctly. The patch is still incomplete (mapping of time zone definitions should also use a hard-coded list of Outlook TZIDs; freeing ical strings is completely missing and requires more work to make it compatible with the old and new memory ownership of libical). I'll post it for review and further discussion anyway. I don't have a solution for the third problem.
Created attachment 109535 [details] [review] adds time zone checking to the libecal and the file backend (note that it won't compile as it is: a NULL arg must be added after toplevel_comp in the e_cal_check_timezones() call due to a prototype change) The most recent implementation of the e_cal_check_timezone() call can be found in the SyncEvolution repository. These two files need to be placed in the calendar/libecal directory: https://zeitsenke.de/websvn/filedetails.php?repname=SyncEvolution&path=%2Ftrunk%2Fsrc%2Fe-cal-check-timezones.c&rev=0&sc=0 https://zeitsenke.de/websvn/filedetails.php?repname=SyncEvolution&path=%2Ftrunk%2Fsrc%2Fe-cal-check-timezones.h&rev=0&sc=0
I can understand the solution well. But I do not want to store conflicting timezone definitions with new tzids. The problem of conflicting timezones definitions with same tzid arises only with respect to the exchange timezones. Other than exchange I have not observed this problem with events received from other clients. The timezone locations from other clients such as google can be mapped easily with system timezones. I would like the solution to be completely in the backend. We need to pickup the timezone always from the system by mapping the exchange timezones with system ones. The timezones are added using e_cal_backend_add_timezone and retrievied using e_cal_backend_get_timezone from the backend. The problem can be solved by implementing the following at the backend, In backend_add_timezone • check if the timezone can be mapped with system zone at _e_cal_backend_add_timezone. e_cal_match_tzid and e_cal_match_location needs to be used here. • If it can be mapped do not call the derived class function to add the timezone to cache • If it cant be mapped, add it to cache In backend_get_timezone • Return the system timezone if the tzid can be mapped with any of our system timezones • Else get the timezone from the cache calling the derived class function In receive_objects, * Always call the base class function to add the timezones to cache. Now, the clients can always get the right timezone using e_cal_get_timezone. There would be one problem though. The older events may not appear properly because of the issue 3 mentioned at comment #3. Issue:3 needs to be fixed in libical by storing the old rules in VTIMEZONE. I think the problem of older events not appearing properly is not very severe. This can be taken up as a separate task. After implementing the above, we need not have the e_cal_check_timezone at the client. Patrick, What is ur opinon about this ?
> But I do not want to store conflicting > timezone definitions with new tzids. Why? > In backend_add_timezone > • check if the timezone can be mapped with system zone at > _e_cal_backend_add_timezone. e_cal_match_tzid and e_cal_match_location needs to > be used here. > • If it can be mapped do not call the derived class function to add the > timezone to cache > • If it cant be mapped, add it to cache If it was mapped, how does a client program know which TZID it was mapped to? You seem to suggest that the VEVENT should be stored with its original TZID. I find that troublesome: it makes the .ics file inconsistent. If the mapping from TZID to system time zone ever has to be changed, then we end up with VEVENTs which either have no or not the right VTIMEZONE. My other objection is that you assume that the mapping can be done in all cases. I doubt that. Do you have a full list of Outlook TZIDs and the corresponding Olson location? If there are TZIDs which cannot be mapped, then we need a plan B for handling collisions. My plan B is renaming the VTIMEZONEs. How do you suggest to handle this? > I think the problem of older events not appearing properly is not very severe. I think this *is* a severe problem (but of course I might miss something): imagine that system time zone definitions get updated now to accomodated for a fictional summer saving change in 2009. My impression is that Evolution will then use the 2009 rules for all remaining events in 2008. Is that correct? In that case people will again miss their meetings, like some did when summer saving time started in 2008. > This can be taken up as a separate task. Even if I miss something and Evolution displays all future events correctly, I think we should at least have a plan how we are going to address the problem. With renaming of VTIMEZONEs, all one has to do is improve the support for reading and using system time zone definitions. Without renaming, you also have to implement merging of timezone definitions in those cases where mapping fails. This merging is impossible to get right for the two example VTIMEZONEs that Outlook used, plus it is more code to be written.
One more comment on this: > The problem of conflicting timezones > definitions with same tzid arises only with respect to the exchange timezones. "Only Exchange" would be bad enough to get right in all cases, but there are many programs out there and some of them are bound to cause the same problem. If you don't believe that, then I can do a search in my mail archive at home (I'm traveling right now, so I cannot do it right away) and most likely will find one non-Microsoft example of a TZID which would require hard-wired mapping. I remember seeing that a while ago in a report by a user.
> > But I do not want to store conflicting > > timezone definitions with new tzids. > > Why? I was a bit brief here, sorry for that. The full question would have been: why do you not want to do that - are there technical issues that I fail to fully understand at this time?
As I keep thinking about this I run into more issues, so let me add one more thought which strengthens my uneasiness about storing the original TZID without the corresponding VTIMEZONE. [...] > If it was mapped, how does a client program know which TZID it was mapped to? > You seem to suggest that the VEVENT should be stored with its original TZID. I > find that troublesome: it makes the .ics file inconsistent. If the mapping from > TZID to system time zone ever has to be changed, then we end up with VEVENTs > which either have no or not the right VTIMEZONE. Assume that a user tries out Evolution trunk and creates an .ics file which has a VEVENT with the original TZID, but without the VTIMEZONE because that was mapped. This user cannot go back to using the stable Evolution (or the one which is installed as part of his distribution) because that Evolution doesn't have the time zone mapping yet. Users also cannot move the .ics file between computers with such different Evolution versions. This might not be a common use case or not "officially supported", but it currently works and I do it myself when traveling.
(In reply to comment #6) > > But I do not want to store conflicting > > timezone definitions with new tzids. > > Why? It might become difficult maintain later. It would be better if we have one list of tzids. > > > In backend_add_timezone > > • check if the timezone can be mapped with system zone at > > _e_cal_backend_add_timezone. e_cal_match_tzid and e_cal_match_location needs to > > be used here. > > • If it can be mapped do not call the derived class function to add the > > timezone to cache > > • If it cant be mapped, add it to cache > > If it was mapped, how does a client program know which TZID it was mapped to? The client need not know that. It can just ask e_cal_get_timezone to get the right timezone from the backend. > You seem to suggest that the VEVENT should be stored with its original TZID. I > find that troublesome: it makes the .ics file inconsistent. If the mapping from > TZID to system time zone ever has to be changed, then we end up with VEVENTs > which either have no or not the right VTIMEZONE. Yes VEVENT would be stored with the original tzid. Changing the tzid of the VEVENT may be ok in case of file backend, but may not be ok in case of exchange-connector as it needs be syncronized with the server. If we change the mapping, the event probably would use a different timezone and I assume we wont remove any mapping. > > My other objection is that you assume that the mapping can be done in all > cases. I doubt that. Do you have a full list of Outlook TZIDs and the > corresponding Olson location? If there are TZIDs which cannot be mapped, then > we need a plan B for handling collisions. My plan B is renaming the VTIMEZONEs. > How do you suggest to handle this? I think all the outlook timezones can be mapped. The timezones have been already mapped for exchange 2007 support which requires it. I will add suman an jhonny in CC'list who are involved in mapi work. > > > I think the problem of older events not appearing properly is not very severe. > > I think this *is* a severe problem (but of course I might miss something): > imagine that system time zone definitions get updated now to accomodated for a > fictional summer saving change in 2009. My impression is that Evolution will > then use the 2009 rules for all remaining events in 2008. Is that correct? In > that case people will again miss their meetings, like some did when summer > saving time started in 2008. As far I have seen so far, the definition of the latest rule takes current year also into account. So I dont think this might be an issue. Same goes well with the latest rules. The 2008 issue was that evolution was still using the old rules. If it had used the latest rules, we would not have had the issue. > > > This can be taken up as a separate task. > > Even if I miss something and Evolution displays all future events correctly, I > think we should at least have a plan how we are going to address the problem. Once libical is modified to include the historic rules, it should be solved. The tzfile in the system has the historic data. We just dont put in VTIMEZONE. There should be some changes made in exchange-connector too for this to be done. > > With renaming of VTIMEZONEs, all one has to do is improve the support for > reading and using system time zone definitions. Without renaming, you also have > to implement merging of timezone definitions in those cases where mapping > fails. This merging is impossible to get right for the two example VTIMEZONEs > that Outlook used, plus it is more code to be written. I think in case if any other client causes similar issues as outlook, my assumption would fail. You can go ahead with what you have proposed. It would be good to store the conflicting timezones with new tzid's only if they cannot be mapped. As i understand from the code, its being done in that way, so I no issues. It would be good if the check for mapping the timezones is done commonly to all backends rather than putting it in the file backend as mentioned at comment #5.
(In reply to comment #9) > As I keep thinking about this I run into more issues, so let me add one more > thought which strengthens my uneasiness about storing the original TZID without > the corresponding VTIMEZONE. > > [...] > > If it was mapped, how does a client program know which TZID it was mapped to? > > You seem to suggest that the VEVENT should be stored with its original TZID. I > > find that troublesome: it makes the .ics file inconsistent. If the mapping from > > TZID to system time zone ever has to be changed, then we end up with VEVENTs > > which either have no or not the right VTIMEZONE. > > Assume that a user tries out Evolution trunk and creates an .ics file which has > a VEVENT with the original TZID, but without the VTIMEZONE because that was > mapped. This user cannot go back to using the stable Evolution (or the one > which is installed as part of his distribution) because that Evolution doesn't > have the time zone mapping yet. > Users also cannot move the .ics file between computers with such different > Evolution versions. This might not be a common use case or not "officially > supported", but it currently works and I do it myself when traveling. Yes the .ics file has to be exported and it cant be just copied. The reason why I did not want to have multiple tzid's was it would become hard to write a tool to migrate timezones later and we don't have any kind of standard in renaming the tzids. I also did not like the comparison on rrule everytime but there is no other way to differenciate outlook VTIMEZONES. In an ideal situation it would be good to have only one set of tzids. But anyways we never have an ideal situation :-) I hope you can document how the new tzid generation is going to be done in this bug and go ahead with it.
> I think all the outlook timezones can be mapped. The timezones have been > already mapped for exchange 2007 support which requires it. I will add suman an > jhonny in CC'list who are involved in mapi work. Where can I find that code? How can we share this code? Should it be moved into a libecal function eventually? I suppose at least temporarily we'll have to keep two copies around until both are incorporated in the same branch, right?
I think we agree that "plan B" (= renaming TZID if it cannot be mapped and conflicts) is necessary. What I don't quite understand yet is whether that'll be a problem for some backends, in particular the Exchange backend ("Changing the tzid of the VEVENT may be ok in case of file backend, but may not be ok in case of exchange-connector as it needs be syncronized with the server."). I imagine it could be if "FOO" is currently recognized and mapped to the corresponding Exchange time zone, whereas a renamed "FOO 1" wouldn't be recognized. Can someone describe how the Exchange backend currently handles time zones? How are custom VTIMEZONE definitions stored in the Exchange server and retrieved again? Please include pointers into the source code if you can, so that I can look it up. That would help a lot, otherwise I would be in for some searching (works, but is slower). My assumption is that the Exchange backend and Exchange/Outlook work with custom time zone definitions imported from VTIMEZONE. Otherwise there would be a severe interoperability problem. If that assumption holds, renamed VTIMEZONE definitions should work, although avoiding that still would be better. Now regarding the suggestion in comment #5 to do time zone mapping at the layer above the backends: I believe we should add the mapping to system time zones in backend_get_timezone, regardless of what we do about importing events. My reasoning is that a) it "fixes" existing events which users already have in their calendar and b) it helps with backends/clients which are not yet/cannot be changed to fix events when importing them. Now, to get a) right in all cases we need a solution for problem 3. in comment #1. So let's continue the discussion about that: > > > I think the problem of older events not appearing properly is not very severe. > > > > I think this *is* a severe problem (but of course I might miss something): > > imagine that system time zone definitions get updated now to accomodated for a > > fictional summer saving change in 2009. My impression is that Evolution will > > then use the 2009 rules for all remaining events in 2008. Is that correct? In > > that case people will again miss their meetings, like some did when summer > > saving time started in 2008. > As far I have seen so far, the definition of the latest rule takes current year > also into account. So I dont think this might be an issue. Same goes well with > the latest rules. Do you have pointers to the code which implements this behavior? I want to see how it works and what would have to get changed to let libical use the historic and future rules in addition to the current ones. Any thoughts on that?
(In reply to comment #12) > Where can I find that code? code: http://svn.gnome.org/viewvc/evolution-data-server/branches/EXCHANGE_MAPI_BRANCH/calendar/backends/mapi/e-cal-backend-mapi-tz-utils.c?view=markup TZ data for Exchange appointments are stored in a blob. The file indicated above tries to encode/decode the blob while sending/receiving appointments to the Exchange server. Historical timezone information is stored in structures called TZRULE by Outlook/Exchange. These structures are a part of the same blob. Please note that the file indicated above does *not* yet make use of this information to calculate the offsets. It depends on the information provided by libical to compute the start/end times of appointments. mappings: http://svn.gnome.org/viewvc/evolution-data-server/branches/EXCHANGE_MAPI_BRANCH/calendar/backends/mapi/tz-ical-to-mapi?view=markup http://svn.gnome.org/viewvc/evolution-data-server/branches/EXCHANGE_MAPI_BRANCH/calendar/backends/mapi/tz-mapi-to-ical?view=markup A simple one-to-one mapping is impossible to implement. For example, Outlook maps 'Kuala Lumpur, Singapore' to a single GMT offset. Libical has two separate locations for the same (with or without the same offsets is inconsequential).
> TZ data for Exchange appointments are stored in a blob. The file indicated > above tries to encode/decode the blob while sending/receiving appointments to > the Exchange server. Just so that I get this right, this is what will be used for custom VTIMEZONE definitions? Where is the corresponding code in the old Exchange backend? > Please note > that the file indicated above does *not* yet make use of this information to > calculate the offsets. It depends on the information provided by libical to > compute the start/end times of appointments. So if the mapping fails, events are not displayed correctly? I suppose you have it in your roadmap to implement this, because otherwise non-Outlook events stored in Exchange wouldn't be handled correctly, right? Are events created by old Evolution releases affected the same way (those which did not yet use the /softwarestudio.org/Tzfile/ prefix)? Regarding the mapping itself, where do the Exchange strings come from? They all end in "Standard Time" - what about summer saving time? What I also don't understand is how these strings relate to the TZID strings used by Outlook. Take the example events (attachement #109531): they use TZID "GMT -0600 (Standard) / GMT -0500 (Daylight)". When such an invitation is sent to the MAPI backend, how is the time zone handled?
(In reply to comment #15) > Just so that I get this right, this is what will be used for custom VTIMEZONE > definitions? Where is the corresponding code in the old Exchange backend? yep.. This is the data that should be used to generate the custom VTIMEZONE definitions. The old Exchange backend does not manipulate this blob. [This is admittedly a bug. Just to get this documented somewhere, here's how it can be reproduced: ** Premise: All client systems referred to below are set to Asia/Calcutta aka GMT +0530 timezone. 1) Create an appointment on an Exchange 2003 server using Evolution and the old connector. Ensure that the timezone of the appointment is different from the system timezone (i.e. different from Asia/Calcutta.. let it be, say, America/New_York) 2) View the same appointment using Outlook 2003 (which AFAIK does not allow you to change timezones for an appointment). The appointment appears to be created correctly on the server, since the start/end times are all shown correctly. 3) Export the same appointment to a .pst file. 4) Import the same .pst file into Outlook 2007 (preferably configured with the same user account) 5) View the same appointment in Outlook 2007 (which does allow you to change timezones for an appointment). Again, the appointment appears to be created correctly, since the start/end times are all shown correctly. 6) Open the appointment in the appointment-editor and notice the timezone for the same. It would show the system timezone i.e. GMT +0530 instead of GMT -0500. GMT -0500 is for Eastern Time (US & Canada), which covers New_York. ** Expected behavior: The appointment timezone should have shown up as GMT -0500 in Outlook 2007 ] > So if the mapping fails, events are not displayed correctly? I suppose you have > it in your roadmap to implement this, because otherwise non-Outlook events > stored in Exchange wouldn't be handled correctly, right? Are events created by > old Evolution releases affected the same way (those which did not yet use the > /softwarestudio.org/Tzfile/ prefix)? Each of the Exchange timezones are flexibly mapped (using installable files) to a libical timezone. Could you explain when the mapping would fail? > Regarding the mapping itself, where do the Exchange strings come from? They all end in "Standard Time" - what about summer saving time? http://msdn.microsoft.com/en-us/library/ms912053.aspx http://technet2.microsoft.com/WindowsVista/en/library/31f49a21-cfed-4b63-b420-58a9eabbb04e1033.mspx?mfr=true The summer savings are mentioned in and as TZRULEs. This gives a mapping from another application: http://search.cpan.org/src/DROLSKY/DateTime-TimeZone-0.71/lib/DateTime/TimeZone/Local/Win32.pm > What I also don't understand is how these strings relate to the TZID strings > used by Outlook. Take the example events (attachement #109531): they use TZID > "GMT -0600 (Standard) / GMT -0500 (Daylight)". When such an invitation is sent > to the MAPI backend, how is the time zone handled? There are two strings in an Outlook timezone - a display string [typically in the form: (GMT +-XXXX) Location A, Location B, ...] and the internal TZID string. The MAPI backend does not use the display strings. FWIW, the display strings could be literally anything. The MAPI backend looks for the blob and tries to parse it. If there is no blob OR the parsing fails, the backend's default timezone is used instead.
Chenthill, I have a few questions regarding problem #3 and the current code which translates zone information into VTIMEZONE definitions. For reference, that code is contained in src/libical/icaltz-util.c. I see that find_transidx() indeed looks at the current time to identify the transitions which then get turned into RRULEs of STANDARD resp. DAYLIGHT components of a VTIMEZONE. The DTSTART of these components is faked (1970 instead of real time). Why was the rather complex translation into RRULEs used instead of just giving an RDATE? RRULEs have to be in local time whereas RDATE is given in UTC, just like the transitions stored in the zone info (?). The RRULE will be correct only for the current year anyway, won't it? As an example, let's consider Asia/Tehran, which has multiple transitions. From tzdata-2008a (which is the one used on my Ubuntu installation): ... Rule Iran 2005 only - Mar 22 0:00 1:00 D Rule Iran 2005 only - Sep 22 0:00 0 S Rule Iran 2008 only - Mar 21 0:00 1:00 D Rule Iran 2008 only - Sep 21 0:00 0 S Rule Iran 2009 2011 - Mar 22 0:00 1:00 D Rule Iran 2009 2011 - Sep 22 0:00 0 S ... zdump -v Asia/Tehran: ... Asia/Tehran Wed Sep 21 19:30:00 2005 UTC = Wed Sep 21 23:00:00 2005 IRST isdst=0 gmtoff=12600 Asia/Tehran Thu Mar 20 20:29:59 2008 UTC = Thu Mar 20 23:59:59 2008 IRST isdst=0 gmtoff=12600 Asia/Tehran Thu Mar 20 20:30:00 2008 UTC = Fri Mar 21 01:00:00 2008 IRDT isdst=1 gmtoff=16200 Asia/Tehran Sat Sep 20 19:29:59 2008 UTC = Sat Sep 20 23:59:59 2008 IRDT isdst=1 gmtoff=16200 Asia/Tehran Sat Sep 20 19:30:00 2008 UTC = Sat Sep 20 23:00:00 2008 IRST isdst=0 gmtoff=12600 Asia/Tehran Sat Mar 21 20:29:59 2009 UTC = Sat Mar 21 23:59:59 2009 IRST isdst=0 gmtoff=12600 ... Evolution 2.12 turns that into: BEGIN:DAYLIGHT TZNAME:IRDT DTSTART:19700322T010000 RRULE:FREQ=YEARLY;INTERVAL=1;BYDAY=-2SU;BYMONTH=3 TZOFFSETFROM:+0330 TZOFFSETTO:+0430 END:DAYLIGHT Without single-stepping through the code I don't see how "Thu Mar 20 20:30:00 2008" leads to BYDAY=-2SU. Is that really correct? The factor of 8 in set_zone_directory() looks fishy to me. I also don't see the advantage of counting backwards from the end of the month - wouldn't it be easier to always count from the beginning of the month? When the Evolution GUI displays times, does it use the system functions (and thus the complete system time zone definitions) to convert time stamps into local time or does it use libical and its incomplete definitions?
> > Just so that I get this right, this is what will be used for custom VTIMEZONE > > definitions? Where is the corresponding code in the old Exchange backend? > > yep.. This is the data that should be used to generate the custom VTIMEZONE > definitions. > The old Exchange backend does not manipulate this blob. Then how do the Exchange backends currently implement e_cal_add_timezone()? > > So if the mapping fails, events are not displayed correctly? I suppose you have > > it in your roadmap to implement this, because otherwise non-Outlook events > > stored in Exchange wouldn't be handled correctly, right? Are events created by > > old Evolution releases affected the same way (those which did not yet use the > > /softwarestudio.org/Tzfile/ prefix)? > > Each of the Exchange timezones are flexibly mapped (using installable files) to > a libical timezone. Could you explain when the mapping would fail? When Outlook imports a meeting invitation which uses a non-Outlook TZID? I'm just guessing here, but wouldn't that lead to an event where the VTIMEZONE blob has to be parsed and used? > > Regarding the mapping itself, where do the Exchange strings come from? They all end in "Standard Time" - what about summer saving time? > > http://msdn.microsoft.com/en-us/library/ms912053.aspx > http://technet2.microsoft.com/WindowsVista/en/library/31f49a21-cfed-4b63-b420-58a9eabbb04e1033.mspx?mfr=true Let me rephrase my question: when does the Evolution exchange backend see these strings? They don't appear in the VTIMEZONE that I got from Outlook. > > What I also don't understand is how these strings relate to the TZID strings > > used by Outlook. Take the example events (attachement #109531): they use TZID > > "GMT -0600 (Standard) / GMT -0500 (Daylight)". When such an invitation is sent > > to the MAPI backend, how is the time zone handled? > > There are two strings in an Outlook timezone - a display string [typically in > the form: (GMT +-XXXX) Location A, Location B, ...] and the internal TZID > string. The MAPI backend does not use the display strings. FWIW, the display > strings could be literally anything. So you are saying that the mapping is for the internal TZID. But then why does Outlook 2003 send "GMT -0600 (Standard) / GMT -0500 (Daylight)" as TZID? This is a string which is not part of the mapping table. I even doubt that it can be mapped at all, because there can be multiple locations which have -0600 and -0500 as offsets, but switch at different dates. Again, when such a meeting invitation is sent to the MAPI backend, how would it handle the event? > The MAPI backend looks for the blob and tries to parse it. If there is no blob > OR the parsing fails, the backend's default timezone is used instead. Hmm, I guess I'm getting confused now. Previously you said "Please note that the file indicated above does *not* yet make use of this information to calculate the offsets. It depends on the information provided by libical to compute the start/end times of appointments."
As promised, here's the example of a VTIMEZONE which uses yet another TZID: BEGIN:VTIMEZONE TZID:Mountain BEGIN:STANDARD DTSTART:19501105T020000 TZOFFSETFROM:-0600 TZOFFSETTO:-0700 RRULE:FREQ=YEARLY;INTERVAL=1;BYMINUTE=0;BYHOUR=2;BYDAY=1SU;BYMONTH=11 END:STANDARD BEGIN:DAYLIGHT DTSTART:19500312T020000 TZOFFSETFROM:-0700 TZOFFSETTO:-0600 RRULE:FREQ=YEARLY;INTERVAL=1;BYMINUTE=0;BYHOUR=2;BYDAY=2SU;BYMONTH=3 END:DAYLIGHT END:VTIMEZONE As far as I can tell, it might of have originated from Lotus Notes. Another user sent this one: TZID=(GMT) Greenwich Mean Time %3A Dublin%2C Edinburgh%2C Lisbon%2C London The %2C might be quotation artifacts.
The original problem description was "Evolution's time zone handling fails to handle the fact that time zone definitions can change." After some experiments with Outlook I have come to the conclusion that the same problem also exists even if time zone definitions don't change: Outlook (at least 2003) seems to generate the TZID and VTIMEZONE dynamically for each event. The effect is that events scheduled in different locations with different time zone rules end up with the same TZID. Steps to reproduce: - set Windows time zone to "(GMT -04:00) Atlantic Time (Canada)" - create a recurring event starting on March 13th 2009, 8:00am - set Windows time zone to "(GMT -04:00) Santiago" - restart Outlook (might not be necessary, not sure) - create a recurring event starting on March 13th 2009, 8:00am Expected behavior: - on March 13th 2009, 8:00am Atlantic time corresponds to 8:00am Santiago time => events occur at the same time - on March 20th 2009, 8:00am Atlantic time corresponds to 7:00am Santiago time => events occur at differnet times This is what Outlook displays. Actual behavior: - After importing the meeting invitations, Evolution always shows them at the same time. I'm going to attach these meeting invitations, too. Suman, note that again there is no trace left in the VTIMEZONE which links the rules back to the original time zone ID strings, i.e., the mapping in the MAPI backend won't work. Because the TZID is identical, any approach based exclusively on the TZID will fail. We need plan B (storing conflicting VTIMEZONEs under modified TZIDs, agreed?) and all backends must implement storing custom time zones (supported?).
Created attachment 110312 [details] recurring meeting scheduled relative to Atlantic Time
Created attachment 110313 [details] recurring meeting scheduled relative to Santiago Time
Regarding comment #20: I had imported the meetings into a file backend. This just confirmed the already observed behavior. Now I also tested with the Exchange backend. Steps to reproduce: - open Exchange calendar where Outlook created the events - look at March 13th 2009 Expected behavior: - as described for Outlook Actual behavior: - as with the file backend, both events are shown at the same time ------ Steps to reproduce: - create new Exchange calendar - import meetings into that calendar with Evolution - look at March 13th 2009 Expected behavior: - as described for Outlook Actual behavior: - as before, both events are shown at the same time ----- On the bright side, the custom time zone definitions seem to be preserved by Evolution-Exchange, so the renaming trick should work.
(In reply to comment #17) > Chenthill, I have a few questions regarding problem #3 and the current code > which translates zone information into VTIMEZONE definitions. For reference, > that code is contained in src/libical/icaltz-util.c. > > I see that find_transidx() indeed looks at the current time to identify the > transitions which then get turned into RRULEs of STANDARD resp. DAYLIGHT > components of a VTIMEZONE. The DTSTART of these components is faked (1970 > instead of real time). > > Why was the rather complex translation into RRULEs used instead of just giving > an RDATE? RRULEs have to be in local time whereas RDATE is given in UTC, just > like the transitions stored in the zone info (?). This is done for outlook compatibility. As I had said earlier, this needs to be changed to include historic rules. Exchange does not like multiple RDates.So this requires changes to be made in libical as well as in exchange connector. > > The RRULE will be correct only for the current year anyway, won't it? We will always take the latest transistion which will be for the current year. > > As an example, let's consider Asia/Tehran, which has multiple transitions. > From tzdata-2008a (which is the one used on my Ubuntu installation): > ... > Rule Iran 2005 only - Mar 22 0:00 1:00 D > Rule Iran 2005 only - Sep 22 0:00 0 S > Rule Iran 2008 only - Mar 21 0:00 1:00 D > Rule Iran 2008 only - Sep 21 0:00 0 S > Rule Iran 2009 2011 - Mar 22 0:00 1:00 D > Rule Iran 2009 2011 - Sep 22 0:00 0 S > ... > zdump -v Asia/Tehran: > ... > Asia/Tehran Wed Sep 21 19:30:00 2005 UTC = Wed Sep 21 23:00:00 2005 IRST > isdst=0 gmtoff=12600 > Asia/Tehran Thu Mar 20 20:29:59 2008 UTC = Thu Mar 20 23:59:59 2008 IRST > isdst=0 gmtoff=12600 > Asia/Tehran Thu Mar 20 20:30:00 2008 UTC = Fri Mar 21 01:00:00 2008 IRDT > isdst=1 gmtoff=16200 > Asia/Tehran Sat Sep 20 19:29:59 2008 UTC = Sat Sep 20 23:59:59 2008 IRDT > isdst=1 gmtoff=16200 > Asia/Tehran Sat Sep 20 19:30:00 2008 UTC = Sat Sep 20 23:00:00 2008 IRST > isdst=0 gmtoff=12600 > Asia/Tehran Sat Mar 21 20:29:59 2009 UTC = Sat Mar 21 23:59:59 2009 IRST > isdst=0 gmtoff=12600 > ... > > Evolution 2.12 turns that into: > > BEGIN:DAYLIGHT > TZNAME:IRDT > DTSTART:19700322T010000 > RRULE:FREQ=YEARLY;INTERVAL=1;BYDAY=-2SU;BYMONTH=3 > TZOFFSETFROM:+0330 > TZOFFSETTO:+0430 > END:DAYLIGHT > > Without single-stepping through the code I don't see how "Thu Mar 20 20:30:00 > 2008" leads to BYDAY=-2SU. Is that really correct? We have tried to be in sync with what outlook provides. The rrules can be set right only if we can maintain the historic rules. I think this should be done. > The factor of 8 in set_zone_directory() looks fishy to me. I also don't see the > advantage of counting backwards from the end of the month - wouldn't it be > easier to always count from the beginning of the month? I dont get you. > > When the Evolution GUI displays times, does it use the system functions (and > thus the complete system time zone definitions) to convert time stamps into > local time or does it use libical and its incomplete definitions? It uses the libical timezones.
(In reply to comment #20) > The original problem description was "Evolution's time zone handling fails to > handle the fact that time zone definitions can change." After some experiments > with Outlook I have come to the conclusion that the same problem also exists > even if time zone definitions don't change: Outlook (at least 2003) seems to > generate the TZID and VTIMEZONE dynamically for each event. The effect is that > events scheduled in different locations with different time zone rules end up > with the same TZID. > > Steps to reproduce: > - set Windows time zone to "(GMT -04:00) Atlantic Time (Canada)" > - create a recurring event starting on March 13th 2009, 8:00am > - set Windows time zone to "(GMT -04:00) Santiago" > - restart Outlook (might not be necessary, not sure) > - create a recurring event starting on March 13th 2009, 8:00am > > Expected behavior: > - on March 13th 2009, 8:00am Atlantic time corresponds to 8:00am Santiago time > => events occur at the same time > - on March 20th 2009, 8:00am Atlantic time corresponds to 7:00am Santiago time > => events occur at differnet times > > This is what Outlook displays. > > Actual behavior: > - After importing the meeting invitations, > Evolution always shows them at the same time. > > I'm going to attach these meeting invitations, too. Suman, note that again > there is no trace left in the VTIMEZONE which links the rules back to the > original time zone ID strings, i.e., the mapping in the MAPI backend won't > work. This is a very nice catch and very much bad from outlook side. We have to handle this anyways. > > Because the TZID is identical, any approach based exclusively on the TZID will > fail. We need plan B (storing conflicting VTIMEZONEs under modified TZIDs, > agreed?) and all backends must implement storing custom time zones > (supported?). I think we need to go with plan B then. One thing which needs to be taken care here is that the renamed tzid's should be synced with server in case of exchange-connector. W.r.t storing historic rules in libical, I think suman can take up this work.
> > Why was the rather complex translation into RRULEs used instead of just giving > > an RDATE? RRULEs have to be in local time whereas RDATE is given in UTC, just > > like the transitions stored in the zone info (?). > This is done for outlook compatibility. As I had said earlier, this needs to be > changed to include historic rules. Exchange does not like multiple RDates. My question was about something else: why do the generated VTIMEZONE use RRULE instead of RDATE? Does Exchange also choke on a single RDATE? RRULE might be preferable if and only if we are limited to storing one set of rules because then there is at least a chance that we get the other years right. If we store multiple rules, then with RDATE we can directly store the original information without some complex (and potentially buggy, see below) translation step. > > The RRULE will be correct only for the current year anyway, won't it? > We will always take the latest transistion which will be for the current year. I'm not sure we talk about the same thing, so let me try again: do we agree that the generated VTIMEZONE are only guaranteed to be correct for the current year and may be incorrect for the next year? > > As an example, let's consider Asia/Tehran, which has multiple transitions. [...] > > Without single-stepping through the code I don't see how "Thu Mar 20 20:30:00 > > 2008" leads to BYDAY=-2SU. Is that really correct? We have tried to be in sync with what outlook provides. The rrules can be set right only if we can maintain the historic rules. I think this should be done. My question wasn't about using RRULEs here. I'm concerned that the generated RRULE in the example might be faulty due to a bug in the code. The transition occurs on Thursday (UTC) or Friday (local time) - why does the RRULE list Sunday? Is that correct or wrong? > > When the Evolution GUI displays times, does it use the system functions (and > > thus the complete system time zone definitions) to convert time stamps into > > local time or does it use libical and its incomplete definitions? > It uses the libical timezones. That makes it even more important to fix the lack of historic *and* future time zone rules.
As far as I have seen the timezones generated by outlook does not use RDATE at all. They just use the rrule. Thats the reason the RDATE was never used at all. Perhaps the old timezones which are in zoneinfo directory under libical which were generated using olson database never used RDATE's too. I have not tried sending single RDATE with exchange server. The information in the example generated based on the latest rule seems wrong. With storing the timezone definitions with only one RRULE, I don think we can always have accurate data. But I wonder what outlook generates since I verified that most of the timezones generated were similar to what outlook generated. The probable solution which I see is storing the historical data using multiple RDATE/RRULES which should have accurate information and stripping the incompatible part while sending it to exchange server. You can complete the conflicting timezone part and suman can take up the libical work of storing the historical data (all rrules including future) in timezones. I think this would solve the timezone issues completely.
> I have not tried sending single RDATE with exchange server. It might work. But you are right, if all other VTIMEZONE definitions use RRULE then perhaps it is prudent to do likewise simply to avoid running into broken RDATE handling. > The information in the example generated based on the latest rule seems wrong. > With storing the timezone definitions with only one RRULE, I don think we can > always have accurate data. I still don't get it, sorry. Did the "transition to RRULE conversion" code work correctly in this example or is it buggy and needs to be fixed? I agree that we cannot get it right for all years with just one rule, but for one year the conversion can and should be 100% accurate. This is required in any case, because including multiple rules would still be based on that conversion. > But I wonder what outlook generates Can you check it for this example or should I? > The probable solution which I see is storing the historical data using multiple > RDATE/RRULES which should have accurate information and stripping the > incompatible part while sending it to exchange server. We have to strip the incompatible part also before generating meeting invitations. For Evolution itself that is done in the send_objects backend call (right?). For SyncEvolution it is done in e_cal_get_component_as_string(). The problem in both cases is that the purpose of the retrieved data is not clear: some use cases of these calls actually might require full VTIMEZONEs. I cannot think of any, though, so it is probably okay to apply the stripping unconditionally. Note that the stripping should be based on the year of the event(s). There might be multiple years. In that case we could use TZID renaming to avoid the conflicts. The renaming should replace the middle part of the Olson TZID, not attach something to the end, because the receiver might expect the location at the end. > You can complete the conflicting timezone part and suman can take up the > libical work of storing the historical data (all rrules including future) in > timezones. Okay.
[sorry for the late replies, but I'm on vacation :-) ] (In reply to comment #18) > > The old Exchange backend does not manipulate this blob. > Then how do the Exchange backends currently implement e_cal_add_timezone()? The old Exchange connector looks for a VTIMEZONE component within the iCal data. If found, it adds it to the cache. I will explain how MAPI backend should work below. > > > So if the mapping fails, events are not displayed correctly? I suppose you have > > > it in your roadmap to implement this, because otherwise non-Outlook events > > > stored in Exchange wouldn't be handled correctly, right? Are events created by > > > old Evolution releases affected the same way (those which did not yet use the > > > /softwarestudio.org/Tzfile/ prefix)? Yes - if the blob does not have a valid internal TZID, the mapping fails and events are not displayed correctly. Yes and Yes. Non-outlook events would not be handled correctly if they do not manipulate the blob. Ditto for events created by older Evolution releases (even 2.22 does not manipulate the blob). The reason for this is: The timezone blob is the right way to handle timezones for a _MAPI_ client. > Let me rephrase my question: when does the Evolution exchange backend see these > strings? They don't appear in the VTIMEZONE that I got from Outlook. True. The MAPI backend does not use ical data from the server. Instead, it depends on properties (the blob is one of them) for the event. For example, PR_LOCATION is equivalent to the LOCATION icalproperty. > So you are saying that the mapping is for the internal TZID. But then why does > Outlook 2003 send "GMT -0600 (Standard) / GMT -0500 (Daylight)" as TZID? This > is a string which is not part of the mapping table. I even doubt that it can be > mapped at all, because there can be multiple locations which have -0600 and > -0500 as offsets, but switch at different dates. So it seems: Outlook -> export as ical -> TZID is "GMT -0600 (Standard) / GMT -0500 (Daylight)" I do not know why it shows the TZID as above in the ical. I do know that Outlook natively uses MAPI and depends on the blob to determine the timezone information. > Again, when such a meeting invitation is sent to the MAPI backend, how would it > handle the event? It would be mis-handled. > Hmm, I guess I'm getting confused now. Previously you said "Please note that > the file indicated above does *not* yet make use of this information to > calculate the offsets. It depends on the information provided by libical to > compute the start/end times of appointments." :-) Current MAPI backend behavior: Parse the blob -> identify the internal TZID -> use the mapping to map it to a location (i.e. ical TZID) -> do icaltimezone_get_builtin_timezone_from_tzid -> attach it to the component Ideal MAPI backend behavior: Parse the blob -> identify the internal TZID -> parse the TZRULEs (they contain all the data to generate the RRULEs) -> generate a custom VTIMEZONE -> attach it to the component
Created attachment 110600 [details] [review] fix for the Evolution Exchange Connector problems This patch solves the problems mentioned in comment #20 and comment #23. Note that it also touches "send_objects" because that function also imports time zones and works with components using those time zones. I haven't tested that part yet. The file backend didn't implement that call. When is the backend used to send objects and when is Evolution doing that itself?
Comment on attachment 110600 [details] [review] fix for the Evolution Exchange Connector problems The Evolution Exchange patch also contains an additional check to avoid a segfault that I saw. It didn't really solve the problem: if this was triggered, authentication simply fails.
Evolution sends the meeting using IMAP transport, through itip_send_comp (itip-utils.c). I will get the patch reviewed and tested by this week.
(In reply to comment #31) > (From update of attachment 110600 [details] [review] [edit]) > The Evolution Exchange patch also contains an additional check to avoid a > segfault that I saw. It didn't really solve the problem: if this was triggered, > authentication simply fails. > are you speaking about the part in e2k-context.c ?
> > (From update of attachment 110600 [details] [review] [edit] [edit]) > > The Evolution Exchange patch also contains an additional check to avoid a > > segfault that I saw. It didn't really solve the problem: if this was triggered, > > authentication simply fails. > > > > are you speaking about the part in e2k-context.c ? Yes; please ignore it, I'll clean up the patch.
I just had a somewhat strange observation: - import event with TZID which needs to be mapped into patched file backend - save the event from the calendar view - look at the saved event - look at the .evolution/calendar/local/*/calendar.ics Behavior: - the calendar.ics has the mapped TZID, as expected - the saved event still has the original TZID, i.e., it is different from the actually stored data Restarting Evolution and EDS "fixed" this issue: saving the event again correctly uses the mapped TZID. Does Evolution perhaps cache the imported event somewhere?
Regarding Microsoft TZIDs: it seems that meeting invitations sent by Outlook via SMTP use TZIDs which follow the documentation mentioned before. Invitations transmitted via Exchange are converted by the Exchange server to iCalendar 2.0 and contain constructed TZIDs; these are the ones that I have seen (and given as example) so far. This implies that in some (but not all) cases mapping of Microsoft TZID to internal TZIDs is possible. I haven't implemented this yet. Another open issue is the mapping to system time zones in backend_get_timezone. This should be done together with the switch to "complete" internal time zones.
> I will get the patch reviewed and tested by this week. I just committed another update of e-cal-check-timezones.c: if a system time zone is mapped to, that definition gets added to the VCALENDAR. This ensures that the VCALENDAR itself works independently from any auxiliary system time zones. This is important because a) it allows downgrading to a patched 2.22.x which uses /Tzfile/ in TZIDs to an Evolution < 2.12 which uses other TZIDs and b) upgrading to a new TZID scheme doesn't require mapping. Please review the patch/source as it is right now. It already works as it is, even without the missing pieces that I mentioned in comment #36. FYI, I'll be away for about three weeks starting the coming Friday.
Created attachment 110870 [details] Outlook meeting invitation This is an example for an Outlook meeting invitation, courtesy of Mark Swanson from ScheduleWorld. This one uses a TZID which in theory could be mapped to an Olson time zone; in practice the string is different from any of those mentioned by Suman, in particular it doesn't contain the "Standard Time" part that I was suspicious of before. Not all is lost: the TZID seems to be derived from the display name of Microsoft time zones. One could extend the mapping code to recognize those. However, I am giving up on mapping Microsoft TZIDs for now. If someone wants to give it a shot, feel free to fill in the TODO part of e-cal-check-timezone.c.
In comment #17 I expressed some doubts about the correctness of libical's system time zone to VTIMEZONE conversion code. I used Asia/Tehran as example: in that example the transition in spring 2008 takes place Mar 21, at a time which is on Friday (UTC) resp. Saturday (Tehran local time). Evolution puts the transition on the second last Sunday of the month. Here's the VTIMEZONE generated by Exchange for that zone: BEGIN:VTIMEZONE TZID:GMT +0330 (Standard) / GMT +0330 (Daylight) BEGIN:STANDARD DTSTART:16010101T000000 TZOFFSETFROM:+0330 TZOFFSETTO:+0330 END:STANDARD BEGIN:DAYLIGHT DTSTART:16010101T000000 TZOFFSETFROM:+0330 TZOFFSETTO:+0330 END:DAYLIGHT END:VTIMEZONE This is bogus, so we cannot use it to check whether the libical code does the right thing or not. I still think that it is faulty and needs to be fixed.
e_cal_check_timezones() avoids time zone definition conflicts by appending a space followed by a sequence number. It starts counting at 1. I updated e_cal_match_tzid() so that it does the matching with and without such a suffix. The rationale is that currently Exchange time zones are handled by renaming them. In some future version of Evolution it might be possible to map them - but only without the suffix.
Created attachment 110967 [details] [review] replace time zone definitions when exporting events As explained before, there are reasons why calendars might contain events which use an old and/or incomplete time zone definition: * created with old Evolution release which did not yet use system time zones * events created with other programs with TZIDs which could not be mapped yet when the event was imported This patch changes e_cal_get_timezone() so that if possible, the returned time zone definition is taken from the system time zone database. Care was taken to return the time zone with the TZID requested by the caller; otherwise the TZID in a VEVENT would not match with the TZID in the system time zone. I have checked a few situations and they all went through e_cal_get_timezone(): * exporting an event in SyncEvolution (via e_cal_component_as_string()) * saving an event in Evolution (also uses e_cal_component_as_string()) * sending an invitation in Evolution (via itip-util code) The advantage of this patch is that updating system time zone information has an effect on existing events. The drawback is that for historic events the attached VTIMEZONE might be correct while the one generated by the current Evolution for the current year is not. This drawback would go away as soon as the generated VTIMEZONEs are complete. But until then the drawback of using an outdated definition for a recurring event is probably worse, so the patch is IMHO an improvement. A (positive) side effect of the patch is that the backend is not called for system time zones because they go through the new code patch which takes the definition from libical directly.
I'll be out of town until June 6th. If someone has the time, please add Exchange TZID mapping to e-cal-check-timezone.c, review the proposed changes and let me know what you want to have changed when I come back. My question (originally posted to the developers list) about out-of-memory handling still stands; there's an open TODO in e-cal-check-timezone.c regarding that. Someone familiar with the other backends (in particular Groupware) might have a look at what it takes to apply the same time zone handling in those backends; I don't want to touch code which I cannot test. I consider this code ready for inclusion in the 2.22.x branch as it is right now, unless the required testing shows up something (my latest changes did not yet go through the regression testing of SyncEvolution). Even without any further feature changes (like Exchange mapping and complete VTIMEZONEs) it solves several problems which currently bite users, therefore I would like to see it included in the next bug fix update. Hopefully distributions which currently shop 2.22 (like Ubuntu 8.04) will be able to provide an update.
Created attachment 110972 [details] [review] replace time zone definitions when exporting events This version of the patch has passed the SyncEvolution regression testing. One uninitialized memory reference was found and fixed (local status variable not initialized), but apart from that all worked okay.
*** Bug 526964 has been marked as a duplicate of this bug. ***
Am starting to review/test the patch today. Sorry for the delay, I was not able to get into this due to some other work.
(In reply to comment #39) > In comment #17 I expressed some doubts about the correctness of libical's > system time zone to VTIMEZONE conversion code. I used Asia/Tehran as example: > in that example the transition in spring 2008 takes place Mar 21, at a time > which is on Friday (UTC) resp. Saturday (Tehran local time). Evolution puts the > transition on the second last Sunday of the month. > > Here's the VTIMEZONE generated by Exchange for that zone: > > BEGIN:VTIMEZONE > TZID:GMT +0330 (Standard) / GMT +0330 (Daylight) > BEGIN:STANDARD > DTSTART:16010101T000000 > TZOFFSETFROM:+0330 > TZOFFSETTO:+0330 > END:STANDARD > BEGIN:DAYLIGHT > DTSTART:16010101T000000 > TZOFFSETFROM:+0330 > TZOFFSETTO:+0330 > END:DAYLIGHT > END:VTIMEZONE > > This is bogus, so we cannot use it to check whether the libical code does the > right thing or not. I still think that it is faulty and needs to be fixed. > Patrick, I just verified the code while debugging another issue. We don't really take the latest rule. The check is made between the current time and the transitions that follow. We take the closest updated transition from the current time. So what the above VTIMEZONE shows is that we have the updated VTIMEZONE to suit the changes for next coming year. Since the transistion which happens in the march is a bit old (compared with the current time which is May when the comment was made) , we dont consider that. The point here as we discussed earlier is, the old appointments will not appear in proper time, but the upcoming appointments would appear in the right time. For the old appointments to appear properly the history should be stored in VTIMEZONE component.
> So what the above VTIMEZONE shows is that we have the updated > VTIMEZONE to suit the changes for next coming year. Since the transistion which > happens in the march is a bit old (compared with the current time which is May > when the comment was made), we dont consider that. Okay, now I understand. My (incorrect) expectation was that for an event created in 2008, also the rules for 2008 would be included. Now I see how it really works and agree that as long as we are limited to one set of rules the current behavior makes sense. Thanks for explaining it.
Do we agree that this shall go into the next 2.22.x update once it passes the review? When is that update scheduled? Chentill, how is that review going?
The patch looks good. Some comments, Please remove the g_assert statements. In the function patch_ids, ICAL_ANY_PARAMETER can be changed to ICAL_TZID_PARAMETER to fetch the tzid's from properties. e_cal_tzlookup_ecal e_cal_tzlookup_icomp It would be good to use the right argument types instead of void* We have the licensing for all the files under calendar as LGPL. So this file has to adher to the same. I think the copyrights should be assigned to Novell (not sure). Srini would be a better person to comment on this. Since this patch adds new API's, I dont think this can be taken into stable branch.
One more small change which needs to be made is in the function definition style. icaltimezone *e_cal_tzlookup_ecal(const char *tzid, const void *custom, GError **error) Needs to be same as in other files so that doxygen or some doc tools can pick them up. Please add some comments to the API's.
> Since this patch adds new API's, I dont think this can be taken into stable > branch. If the upstream developers don't put this patch into 22.2.x, how is Ubuntu going to fix this problem in their 8.04 LTS release? Same with Evolution 2.22.x in the next Debian release. Not sure about Fedora/Red Hat/Suse... I think it would be better to accept this API *extension* (not API *change*!) into the stable branch in order to fix this problem without forcing distributors to do uncoordinated backports. Note that this is an extension because all the old calls continue to work as they are. Therefore the library versions can be incremented without breaking backwards compatibility (as done during the libical mem fixes). > e_cal_tzlookup_ecal > e_cal_tzlookup_icomp > It would be good to use the right argument types instead of void* This will require casting the function pointer into the required callback type when using the functions. If the callback prototype changes in the future and these calls are accidentally not updated, the compiler won't complain. That's why I wrote the code as it is now. Considering this, do you still want me to change the type? It's a minor issue and I'll change it if you want me to. Regarding the other comments: I'll change the patch and source files as soon as possible. My question from the mailing list about handling out-of-memory situations is still unresolved. I have all the necessary checks in place, but there's a big TODO at the location where a GError needs to be setup if that actually happens. What error codes should be used in such a case? If allocating the GError fails, how should the program be aborted?
> Please remove the g_assert statements. Done. > In the function patch_ids, ICAL_ANY_PARAMETER can be changed > to ICAL_TZID_PARAMETER to fetch the tzid's from properties. Done. > e_cal_tzlookup_ecal > e_cal_tzlookup_icomp > It would be good to use the right argument types instead of void* Not done, see my other comment for the reasoning. Please let me know if you want me to change it to get the patch accepted. > We have the licensing for all the files under calendar as LGPL. So this file > has to adher to the same. I think the copyrights should be assigned to Novell > (not sure). I had already signed a contributor agreement with Novell a while ago and just assigned the copyright to Novell. > One more small change which needs to be made is in the function definition > style. Done (moved to .c file, changed formatting from Doxygen to the format found for the other functions). I haven't checked whether I got the format right. I also implemented out-of-memory error handling. If allocating a GError (g_error_new(E_CALENDAR_ERROR, E_CALENDAR_STATUS_OTHER_ERROR, "out of memory")) fails, the process is aborted by calling g_error(). After making these changes I recompiled as part of libecal and retested that renaming TZIDs still works. e-cal-check-timezones.[ch] are committed as part of SyncEvolution SVN revision 625.
Created attachment 112638 [details] [review] adapts versioning of libecal to the *extended* (not *change*) API This is the libecal built before this patch: lrwxrwxrwx 1 patrick patrick 20 2008-06-12 21:39 libecal-1.2.so -> libecal-1.2.so.7.1.0 lrwxrwxrwx 1 patrick patrick 20 2008-06-12 21:39 libecal-1.2.so.7 -> libecal-1.2.so.7.1.0 -rwxr-xr-x 1 patrick patrick 2040923 2008-06-12 21:39 libecal-1.2.so.7.1.0 With this patch we get: -rwxr-xr-x 1 patrick patrick 1245 2008-06-12 22:11 /scratch/work/evolution-trunk/lib/libecal-1.2.la lrwxrwxrwx 1 patrick patrick 20 2008-06-12 22:11 libecal-1.2.so -> libecal-1.2.so.7.2.0 lrwxrwxrwx 1 patrick patrick 20 2008-06-12 22:11 libecal-1.2.so.7 -> libecal-1.2.so.7.2.0 -rwxr-xr-x 1 patrick patrick 2040923 2008-06-12 22:11 libecal-1.2.so.7.2.0 Any binary requiring libecal-1.2.so.7 continues to work.
2.22.3 release is scheduled for June 30, see http://live.gnome.org/TwoPointTwentythree I consider this as a GNOME target blocker, too - keeping the 2.22.x GNOME milestone.
Patrick, I am ok with keeping the void * for the callbacks. Using E_CALENDAR_STATUS_OTHER_ERROR is better as of now since we dont have any other relevant errors for handling out-of-memory errors. Its also important to extend the patch to exchange connector since most of the users use exchange connector for accepting the meetings than the personal calendar. I would let srini decide on updating the library versions for the stable branch, as iirc we had a few issues in updating the same during the past.
> Its also important to > extend the patch to exchange connector since most of the users use exchange > connector for accepting the meetings than the personal calendar. This is already done for a while, see the patch in comment #30. The Groupware backend hasn't been patched because I don't know it and cannot test it.
The patch done at comment #30 was for the local backend (file). By exchange connector I meant, http://svn.gnome.org/viewvc/evolution-exchange/. Probably suman can take that up later. I have already made a patch for exchange-connector to display the upcoming appointments properly. The old appointments though will not appear properly. Bharath will be committing the patch this week with some other ones which were missed from opensuse.
Created attachment 112819 [details] [review] fix for the Evolution Exchange Connector problems I had fixed the Evolution Exchange connector, but then must have mixed up the diffs. Here's the patch again. This patch solves the problems mentioned in comment #20 and comment #23. Note that it also touches "send_objects" because that function also imports time zones and works with components using those time zones. I haven't tested that part yet.
The patch looks good to commit.
...to stable and trunk?
Yes to stable and trunk. But for stable after getting the approval from the release team as some interfaces are added in libecal. Patrick, one small thing. Please include the libecal linking to CALENDAR_LIBS in configure.in for evolution-exchange.
Comment on attachment 109535 [details] [review] adds time zone checking to the libecal and the file backend (note that it won't compile as it is: a NULL arg must be added after toplevel_comp in the e_cal_check_timezones() call due to a prototype change) libecal change committed on 22.x branch in revision 9023. File backend change committed on 22.x branch in revision 9024. Will merge into trunk and add changelog entries soon.
Comment on attachment 110972 [details] [review] replace time zone definitions when exporting events Committed on 22.x branch in revision 9025.
Comment on attachment 112638 [details] [review] adapts versioning of libecal to the *extended* (not *change*) API Committed to 22.x branch as part of revision 9023. Note that Sri gave his okay in private email; apparently there were precedents where extending the API on the stable branch had been accepted.
Comment on attachment 112819 [details] [review] fix for the Evolution Exchange Connector problems Committed on 22.x branch in revision 1689.
(In reply to comment #64) > Note that Sri gave his okay in private email; apparently there were precedents > where extending the API on the stable branch had been accepted. haven't seen an email to release-team about this, but i'd say it isn't required anyway. generally extending the API without breaking it is allowed but should be avoided. i'm personally fine with this (and the api extension fixes a quite serious bug, so we didn't extend it for fun). @patrick: close as fixed?
Also merged my commits from gnome-2-22 into trunk. Note that my commit messages are quite a bit more verbose than the changes to the ChangeLog; I'm not sure how this is typically handled. The ChangeLog entries are very brief, so I just added a summary there. Regarding the NEWS file: I haven't added anything there. When someone writes it, perhaps it is worth pointing out that updating Evolution will not magically fix existing events. Users have to re-import the meeting invitations they care about if those were not imported correctly earlier. @Andre: this issue has only been resolved for the file and Exchange backend, but not for some of the others like the GroupWise one. There's also the issue with historic and future events which are not necessarily displayed correctly because currently, Evolution only uses a subset of the system's time zone information. In comment #27, Chenthill suggested that Suman could work on that. Finally, mapping of Microsoft TZIDs to Evolution system time zones would be desirable, but isn't implemented yet. My suggestion would be to create a new bug entry for each of these work items and close this one. Getting the GroupWise backend fixed for the stable update might be important; the others can wait for the next major release and/or are too intrusive anyway.
thanks.
The patches are committed. Changing status accordingly. (Why was it undone in the first place? :-) )
@suman: because it's a firefox bug that hasn't been fixed for months? :)
I have created bug #540676, #540680, #540683. Feel free to close this bug or use it as a tracking bug for the other related bugs, if you prefer that. I personally are not sure whether this bug really should be closed before the GroupWise backend is also fixed or verified to not be affected.
Chenthill wrote in comment #57: > I have already made a patch for exchange-connector to display the upcoming > appointments properly. The old appointments though will not appear properly. > Bharath will be committing the patch this week with some other ones which were > missed from opensuse. I haven't seen these commits yet. Chenthill, is the patch for upcoming appointments still necessary? If yes, can you describe the problem that it solves? Should it perhaps be tracked in a separate Bugzilla entry, if it cannot be resolved in time for the 2.22.3 release? What about the other patches from OpenSuse?
Ah no. Its not necessary. Since I thought the patch the exchange part was missing, I was think of committing the patch. You provided the patch for exchange part at comment #58, so it is not necessary any more and will be removed from opensuse as well.
Groupwise backend would not need the fix since all the events are stored/retrieved in UTC timezone on the server. It always uses the libical timezones for the storing the events locally.
> Groupwise backend would not need the fix since all the events are > stored/retrieved in UTC timezone on the server. It always uses the libical > timezones for the storing the events locally. Let's move the discussion regarding Groupwise to bug #540683. I just added a comment there. The gist of it is that IMHO using UTC will not work in all cases.
My work on the original bug is done, but I think it would be good to keep this open as a meta bug. Because Chenthill said that Suman might work one some aspects of the remaining problems (historic times, comment #27) I'm reassigning to him.
I documented the problem with incomplete time zone information in bug #562025 because a Red Hat user ran into it (if my analysis of the problem was right) and we don't seem to have the issue in Bugzilla. https://bugzilla.redhat.com/show_bug.cgi?id=468168
The problem in https://bugzilla.redhat.com/show_bug.cgi?id=468168 also seems to be related to incorrect DTSTART values. This was introduced with the code that converts system time zone information: bug #562028.
Patrick, is this still relevant? I guess libical 1.0 changes timezone handling significantly enough.
(In reply to comment #79) > Patrick, is this still relevant? I guess libical 1.0 changes timezone handling > significantly enough. Yes, I think the Evolution-internal problem (historic times, comment #27) got addressed by the libical 1.0 changed. It opens another can of worms for interoperability (see http://sourceforge.net/p/freeassociation/bugs/95/), but I guess Evolution/EDS can track that separately. Therefore I'm fine with closing this issue here.
OK, thanks.
I forgot to add, the issue with interoperability will be probably fixed in some extent when we'll sanitize the timezone component before passing it to a remote server, though it'll be slightly tricky, due to the need to not modify global builtin timezones returned by libical. Not a big deal, just tricky.