304541 - Timezone picker does nothing.

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Joey Minta]

The timezone picker is brutally honest, telling users that it does nothing. 
Since we actually do look for the calendar.timezone.local pref when guessing the
TZ the user is in, we ought to at least make it possible for them to set it.  A
better way would be the graphical timezone picker in bug 302253.  There are some
concerns about the licensing for the map used there, though.

This bug is simply to make the timezone picker semi-useful for 0.3a1.  Patch to
follow.

Comment 1

[Joey Minta]

Attachment: enable timezone picker

This enables the timezone picker, removing the very long menu-list and instead
dynamically creating it.  This makes it l10n-unfriendly, though.  Should we be
concerned with that?

Also fixes the whitespace, which was done with tabs instead of spaces.

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #1)
> This enables the timezone picker, removing the very long menu-list and instead
> dynamically creating it.  This makes it l10n-unfriendly, though.  Should we be
> concerned with that?

Yes, especially when it is a regression. I think it would be better to just
enable the picker in this bug, and file new bugs on improving it.
A possible solution would be to generate the picker, but keep the translations,
and check if there is a translation for a zone. If not, display english.

> Also fixes the whitespace, which was done with tabs instead of spaces.

That's always a good think to do :)

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 192596 [details] [diff] [review]
enable timezone picker

r-, based on the localization regression.

(Another idea on a better l10n story: Split region from city, so you only need
to translate 'Europe' once)

Comment 4

[Peter Weilbacher]

Seems like this patch here would also get rid of calendar.timezone.default in timezonePrefs.xul. There is another instance of that in resources/content/pref/rootCalendarPref.js. (I guess that both of them are obsolete.)

Comment 5

[Joey Minta]

(In reply to comment #4)
> Seems like this patch here would also get rid of calendar.timezone.default in
> timezonePrefs.xul. There is another instance of that in
> resources/content/pref/rootCalendarPref.js. (I guess that both of them are
> obsolete.)
> 

Yes,calendar.timezone.default is incorrect. calendar.timezone.local is the proper name.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: enable picker

This patch enables the timezone picker. The big part is s/value="/value="/mozilla.org/20050126_1/"/ to give the preference the right value.
A few lines of code were added to set the default to the guessed timezone. It also changes the name of the pref to be set.
I used some shell scripting to verify that the list of timezones in the picker mathces the timezones in zones.tab. I found one error, Europe.Vienna instead of Europe/Vienna. Fixed that.

Comment 7

[Joey Minta]

Comment on attachment 218857 [details] [diff] [review]
enable picker

OK, I'll bite for 0.3a2, as long as we get the new, better prefs in ASAP.

My biggest concern here is that simply showing the panel will set the guessed TZ to be the pref.  Ideally, I think I'd like some checking to make sure that the user really set it.  Any thoughts here?  If you're ok with that, then I am.
+	onload="parent.initPanel('chrome://calendar/content/pref/timezonePrefs.xul');setGuessedTimezone();"
Can you stick a space before setGuessedTimezone?

+function setGuessedTimezone() {
+    var elem = document.getElementById("defaultzone");
+    if (!elem.value) {
+        elem.value = calendarDefaultTimezone();
+    }
+}
+
  </script>
Indenting, and CDATA tags.

r=jminta with the guessing->pref question answered and the above nits.

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: updated patch

I added some tricks with prefstring and onchange, to not save the default value. It will actually get saved, but to an unused pref. Not 100% clean, but the best I could do without spending too much time, given that this code will go away soon.

Comment 9

[Joey Minta]

Comment on attachment 219048 [details] [diff] [review]
updated patch

This looks much better. r=jminta

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in