360259 - prototype event dialog : timezone-support does not work as advertised

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Michael Büttner (no reviews TFN)]

the prototype event dialog supports timezones by providing the option under options->timezone. if this switch is turned on, the timezone of the start- and/or end-time is displayed next to the date/time-picker. changing the date or time works as expected, the time is always expressed in the timezone that is associated with it. as soon as this option is turned off, the times jump back and forth in case any of the related controls is used (date-picker or time-picker).

Comment 1

[Michael Büttner (no reviews TFN)]

Attachment: timezone testfile

this is a manually created ics-file that contains any possible combination of timezones associated with an event, i.e. start=berlin;end=utc, start=ny;end=berlin, etc.

Comment 2

[Michael Büttner (no reviews TFN)]

Attachment: testcases

this file contains a bunch of test-cases i used to test the correct behavior of the dialog with timezone-support enabled and disabled.

Comment 3

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

this patch fixes the above mentioned issues with the date/time-pickers. the previously submitted ics-file was tested against all the test-cases mentioned above.

Comment 4

[Thomas Benisch]

@@ -463,22 +478,30 @@ function updateStartTime()
   }
   
   // jsDate is always in OS timezone, thus we create a calIDateTime
   // object from the jsDate representation and simply set the new
   // timezone instead of converting.
   var kDefaultTimezone = calendarDefaultTimezone();
   var start = jsDateToDateTime(getElementValue(startWidgetId));
   start = start.getInTimezone(kDefaultTimezone);
-  start.timezone = gStartTime.timezone;
+  var menuItem = document.getElementById('menu-options-timezone');
+  if(menuItem.getAttribute('checked') == 'true') {
+    start.timezone = gStartTimezone;
+  }
   gStartTime = start.clone();
-  if(gItemDuration)
+  if(gItemDuration) {
     start.addDuration(gItemDuration);
+    start = start.getInTimezone(gEndTimezone);
+  }
   if(gEndTime) {
-    start.timezone = gEndTime.timezone;
+    var menuItem = document.getElementById('menu-options-timezone');
+    if(menuItem.getAttribute('checked') == 'true') {
+      start.timezone = gEndTimezone
+    }
     gEndTime = start;
   }

Can you explain the single conversion steps in more detail?
In addition, it's a little bit confusing that you use the start variable
for calculating the end time. I would have prefered to use a end variable, e.g.

  var end = start;
  if (gItemDuration) {
    end = end.addDuration(gItemDuration);
  ...

And what about using spaces between if and the opening bracket;-)

-    if(startTime && endTime) {
-      if(startTime.timezone == endTime.timezone || endTime.timezone == "UTC") {
+    if(startTimezone && endTimezone) {
+      if(startTimezone == endTimezone || endTimezone == "UTC") {

Can you elaborate the special case, that the endTimezone is UTC.
Is it also possible, that the startTimezone is UTC?
In addition I prefer to use only one if statement, e.g.

    if(startTimezone && endTimezone &&
       (startTimezone == endTimezone || endTimezone == "UTC")) {

but that's only a matter of style.

Comment 5

[Thomas Benisch]

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

r1=tbe with the questions from comment #4 answered.

Comment 6

[Michael Büttner (no reviews TFN)]

(In reply to comment #4)
> Can you explain the single conversion steps in more detail?
The task is to retrieve the date/time from the appropriate control and create a calIDateTime-object from it.

The first step is to call jsDateToDateTime() which creates the initial calIDateTime-object. Since the control internally is represented by a jsDate-object it doesn't know anything about the timezone that's assigned to it. Therefore the returned calIDateTime-object is always converted to UTC by implicitly assuming the initial date/time-value belongs to the OS timezone. For example, assume we start with 10:00 New York with default calendar timezone and OS timezone set to Europe/Berlin, jsDateToDateTime() returns 9:00 UTC by assuming 10:00 belongs to Berlin, which would mean 9:00 in UTC.

The second step is to convert this back into the default calendar timezone (Europe/Berlin in this example) by calling datetime.getInTimezone(). This results in 10:00 Europe/Berlin (again following this example).

The last step is to force the datetime-object into the final timezone without actually converting anything (datetime.timezone = "New York" in this example). We end with 10:00 New York, which is what the datetime-picker currently shows in this example. This last step (setting the timezone) is skipped if timezone-support is disabled, therefore displaying all times converted to the default timezone.

All of this works only if the default calendar timezone is the same as the OS timezone, at least the UTC-offset needs to be the same, ignoring probably different daylight-saving rules for different timezones.

> Can you elaborate the special case, that the endTimezone is UTC.
> Is it also possible, that the startTimezone is UTC?
> In addition I prefer to use only one if statement, e.g.
An earlier version of the WCAP-provider stored the timezone-information just with the start-date of an item, leaving the end-date as UTC. Therefore after the timezone-feature has been implemented in the dialog old events always showed e.g. Europe/Berlin as start-timezone and UTC as end-timezone (in case both timezones are equal the dialog hides the second one). This was rather unfortunate so I added the special rule that if the start/end-timezones are different and the end-timezone is UTC it gets automatically converted to the start-timezone.

Comment 7

[Michael Büttner (no reviews TFN)]

Checked in on trunk and MOZILLA_1_8_BRANCH