Closed Bug 360259 Opened 13 years ago Closed 13 years ago

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

Categories

(Calendar :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

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

Details

Attachments

(3 files)

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).
Attached file 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.
Attached file 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.
Attached patch patch v1Splinter Review
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.
Attachment #245209 - Flags: first-review?(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 on attachment 245209 [details] [diff] [review]
patch v1

r1=tbe with the questions from comment #4 answered.
Attachment #245209 - Flags: first-review?(thomas.benisch) → first-review+
(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.
Checked in on trunk and MOZILLA_1_8_BRANCH
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.