354845 - Wrong timezone selected in menulist in Preference dialog after restart

Status: VERIFIED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Stefan Sitter [:ssitter]]

Wrong timezone selected in Preference dialog

Steps to Reproduce:
1. Start Sunbird with clean profile
2. Goto Preferences -> Timezone and select your local timezone
3. Press Ok and leave Preferences
4. Quit and restart Sunbird
5. Goto Preferences -> Timezone and check the selected entry

Actual Results:
The first timezone entry 'Africa/Abidjan' is selected. If you now leave Preferences with Ok the wrong timezone will be stored as default.

Expected Results:
The timezone initial set is selected and is not reset.

This is very annoying because you have to reselect the timezone everytime you change something in Preference dialog.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3

Comment 1

[Stefan Sitter [:ssitter]]

Regression:

Works in Sunbird/0.3a2+ (2006-09-27-07)
Fails in Sunbird/0.3RC1 (2006-09-28-10)

Comment 2

[Stefan Sitter [:ssitter]]

The timezone is saved properly after closing the Preference dialog. 
And if I open dialog (wrong entry selected), cancel dialog and reopen dialog the correct entry is selected in menulist dropdown.

So the problem seems to happen only when the Preference dialog is opened the first time after application restart.

Maybe regressed by Bug 333023? CCing Neil Deakin.

Comment 3

[Neil Deakin]

More likely because in timezone.js, the function setTimezone doesn't accomplish anything. Maybe it's trying to set prefValue's value?

Comment 4

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

I poked around in timezones.js, and it seems that the pref gets reset before anything in that file is used. It seems to me that something in the menulist code caused this. Maybe something gets initialized earlier now, making the pref code thing something was selected already?

Comment 5

[Neil Deakin]

I now see that the timezone menulist has an onselect attribute. Menulists now fire a select event when the value is changed (and unfortunately, when changed by script as well, but that's a general issue with various XUL elements). Before the select event wasn't fired so that event handler was never being called.

Comment 6

[Matthew (lilmatt) Willis]

We'll obviously want to figure this out and fix whatever's screwy in timezones.js moving forward.  However, being this close to our release, and not knowing if any other menulists in the app have similar goofs, I'm going to back out the changes from bug 333023 on SUNBIRD_0_3_BRANCH only.  We can come back and address this bug after the 0.3 release.

Clearing blocking0.3+, since this will be "fixed" on 0.3, but we want this bug left open for actually fixing the XUL/js on trunk.

Comment 7

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

Attachment: don't use magic

We don't seem to need all the extra handlers. I think the onselect handler indeed messed up things. The other handlers already didn't do anything. So i removed them all, and now it works.
I left the init handler, because we do want to set the default to the guessed timezone.

Comment 8

[Matthew (lilmatt) Willis]

Comment on attachment 240809 [details] [diff] [review]
don't use magic

>Index: base/content/preferences/timezones.js
>===================================================================
>     },
>-
>-    getTimezoneResult: function() {
>-        var tzMenuList = document.getElementById("calendar.timezone.menulist");
>-        if (tzMenuList.selectedItem != null) {
>-            var value = tzMenuList.value
>-            return value;
>-        }
>-        return undefined;
>-    },
>-
>-    setTimezone: function() {
>-        var prefValue = document.getElementById("calendar.timezone.local").value;
>-        var tzMenuList = document.getElementById("calendar.timezone.menulist");
>-        
>-        prefValue = tzMenuList.value;
>-    }
> };

JS strict warnings nit:
Make sure the last function doesn't have a comma after the close brace.

r=lilmatt with that fixed

Comment 9

[cmtalbert]

Comment on attachment 240809 [details] [diff] [review]
don't use magic

This looks good, with the same nit lilmatt had: remove the comment at the end of the init: function.

Comment 10

[cmtalbert]

(In reply to comment #9)
> (From update of attachment 240809 [details] [diff] [review] [edit])
> This looks good, with the same nit lilmatt had: remove the comment at the end
> of the init: function.
> 
I should have stated, I had dmose's blessing to do the review. (Sorry for the double comment.)

Comment 11

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 12

[Koen Hendrickx]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061031 Calendar/0.4a1

Tried it and it seems to be resolved...

Comment 13

[Joey Minta]

Marking verified thanks to koen

Comment 14

[Stefan Sitter [:ssitter]]

blocking‑calendar0.3.1 is not required. This problem doesn't exist on SUNBIRD_0_3_BRANCH, see Comment #6.

Comment 15

[Martin Schröder [:mschroeder]]

Stefan, I definitely see this problem with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20070130 Calendar/0.3.1pre.

Comment 16

[Stefan Sitter [:ssitter]]

(In reply to comment #15)
You are right:
  WORKS in Calendar/0.3.1pre (2007012905)
  FAILS in Calendar/0.3.1pre (2007013005)

This rather looks like a regression from the tz changes last night that the original problem that caused this bug.

Comment 17

[Matthew (lilmatt) Willis]

This should be fixed on 0.3.1 by bug 368709.  If it isn't after that is checked in, please open a new bug and reference bug 368709 and this one.