129610 - SMAPI preference setting fails to be saved from Mail/Newsgroup window.

Status: VERIFIED FIXED

Description

[Trix Supremo]

The SMAPI preference setting cannot be changed from the Mail/Newsgroup window.
This setting can be changed from the Browser, but not from the Mail/Newsgroup
window.

STEPS:
1. Launch browser, goto preference panel enable, and save preference for SMAPI
to be the default email client.
2. Go back to pref panel to verify the setting was saved.
3. Open Mail/Newsgroups window and attempt to disable SMAPI from the pref panel
and save the changes.
4. Now go back to the pref panel to verify if the setting has been changed.

RESULT:
The prior setting remains, SMAPI was not disabled. The registry also validates
that Netscape 6 is still the default email client.

Comment 1

[Trix Supremo]

nominating for nsbeta1

Comment 2

[Michael Buckland]

Discussed in Mail News bug meeting.  Decided to ADT2 this bug.

Comment 3

[Srilatha Moturi]

Based on conv. with rajiv taking this.

Comment 4

[Rajiv Dayal]

This is a duplicate of what was earlier seen as reported and fixed in bug #
103260. A similiar fix can be applied here with some changes to comply with the
changes done for trunk landing in the pref-mailnewsOverlay.js code. 

Comment 5

[Srilatha Moturi]

Created attachment 78967 [details] [diff] [review]
patch v1

When the preferences is opened from Browser or any other component other than
Mail/News before we call mailnewsOverlayStartup() the Startup() method(which
initalizes hPrefWindow) in pref.xul is getting called. But when we open
preferences from Mail/News mailnewsOverlayStartup() is getting called before
Startup().
http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/content/pref.xul#32


We fixed a similar problem on 0.9.4 branch by doing the callback registration
from Startup in pref-mailnews.js. bug # 103260.
I change the Init() function in pref-mailnews.js since it is not needed anymore
because of the eventlistener code in pref-mailnwsOverlay.js

Comment 6

[Rajiv Dayal]

We looked into alternatives such that we can fix the problem locally in 
pref_mailnewsOverlay.* using an event listener but that is not possible in this 
case since load event does not work as per above comments and no other events 
could be used to get notified onOk.

Srilatha, can we not minimize the changes in pref-mailnews.* by naming 
the 'registerCallback' function in pref-mailnewsOverlay.xul as 'initFunc' 
instead of 'startupFunc' ? Will it lead to same problem ? 

Also please make sure Startup will always be called after hPrefWindow has been 
initialized if you find that changing pref-mailnews.* is necessary and by just 
naming registerCallback() as initFunc will not work.

Comment 7

[Srilatha Moturi]

> Srilatha, can we not minimize the changes in pref-mailnews.* by naming 
> the 'registerCallback' function in pref-mailnewsOverlay.xul as 'initFunc' 
> instead of 'startupFunc' ? Will it lead to same problem ?
Naming the registerCallback function initFunc or startupFunc doesn't make any 
difference. But I named it startupFunc because this is getting called from 
Startup().

Startup() gets called only after hPrefWindow is initialized. 
Startup gets called from nsPrefWindow.onPageLoad
http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/con﷒0﷓
nsPrefWindow.onPageLoad gets called from two places
1) from nsPrefWindow.Init()which gets called only if hPrefWindow is not null
http://lxR.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/con﷒1﷓
http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/con﷒2﷓

2)from InitPanel() only if hPrefWindow is not null
http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/con﷒3﷓

Comment 8

[Rajiv Dayal]

>> Naming the registerCallback function initFunc or startupFunc doesn't make  
>> any difference. But I named it startupFunc because this is getting called  
>> from Startup().

Naming the registerCallback function initFunc instead of startupFunc Will make 
a difference, if you do that you will not have to change the pref-mailnews.js 
and pref-mailnews.xul at all, Only your pref-mailnewsOverlay.* changes could 
suffice.

Anyway as per your comments about Startup function in comment #7 not changing 
pref-mailnews.* will lead to same problem. So your changes are fine.

r=rdayal.

Comment 9

[Scott MacGregor]

I'm slightly confused. Who ends up calling function Startup()? I couldn't see a
caller in the patch. 

Comment 10

[Srilatha Moturi]

This is where Startup() gets called. From nsPrefWindow.onPageLoad().
http://lxr.mozilla.org/seamonkey/source/xpfe/components/prefwindow/resources/content/nsPrefWindow.js#369

Comment 11

[Scott MacGregor]

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

ahh! thanks!

sr=mscott

Comment 12

[Srilatha Moturi]

Fix checked into the trunk.

Comment 13

[scottputterman]

Trix, could you try this out on the trunk.

Comment 14

[Trix Supremo]

verified on trunk builds 2002052008

Comment 15

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
Drivers approval.

Comment 16

[Judson Valeski]

please checkin to the 1.0.1 branch ASAP. once there please remove the
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword.

Comment 17

[Srilatha Moturi]

Fix checked into the branch

Comment 18

[Trix Supremo]

verified on branch build 2002061108 changing keyword fixed1.0.1 to verified1.0.1