Closed Bug 354845 Opened 14 years ago Closed 13 years ago

Wrong timezone selected in menulist in Preference dialog after restart

Categories

(Calendar :: Sunbird Only, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
Sunbird 0.5

People

(Reporter: ssitter, Unassigned)

Details

(Keywords: regression)

Attachments

(1 file)

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
Regression:

Works in Sunbird/0.3a2+ (2006-09-27-07)
Fails in Sunbird/0.3RC1 (2006-09-28-10)
Keywords: regression
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.
Summary: Wrong timezone selected in Preference dialog → Wrong timezone selected in menulist in Preference dialog after restart
Flags: blocking0.3+
More likely because in timezone.js, the function setTimezone doesn't accomplish anything. Maybe it's trying to set prefValue's value?
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?
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.
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.
Flags: blocking0.3+
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → Sunbird 0.5
Attached patch don't use magicSplinter Review
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.
Attachment #240809 - Flags: second-review?(dmose)
Attachment #240809 - Flags: first-review?
Attachment #240809 - Flags: first-review? → first-review?(lilmatt)
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
Attachment #240809 - Flags: first-review?(lilmatt) → first-review+
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.
Attachment #240809 - Flags: second-review?(dmose) → second-review+
(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.)
Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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...
Marking verified thanks to koen
Status: RESOLVED → VERIFIED
Flags: blocking-calendar0.3.1?
blocking‑calendar0.3.1 is not required. This problem doesn't exist on SUNBIRD_0_3_BRANCH, see Comment #6.
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.
(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.
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.
Flags: blocking-calendar0.3.1? → blocking-calendar0.3.1-
You need to log in before you can comment on or make changes to this bug.