99117 - MailNews problems using Turbo Mode

Status: VERIFIED FIXED

(Core Graveyard :: Cmd-line Features, defect)

Description

[Ninoschka Baca]

Branch Build 2001-09-06: WinMe

Overview: When running in Tubo mode a variety of Mail/News problems occur.

1. In a new profile, add one mail account, exit, view the prefs.js file and
duplicate account keys appear or more are created:

a. user_pref("mail.account.account1.identities", "id1");
user_pref("mail.account.account1.server", "server1");
user_pref("mail.account.account7.server", "server2");
user_pref("mail.accountmanager.accounts",
"account1,account2,account3,account4,account5,account6,account7");

b. "account1, account2, account3, account1, account2, account3, account4, account5"

2. I migrated a profile (qatest20), exited, created new profile (new20) and when
Mail opened I tried to configure it for a qatest20 account. An error appears
stating that an account with this name already exists. This error makes no sense
because this is a new profile with no accounts.

Additional Information:
- If Turbo mode is turned off then creating a new account results in a profile
with the correct number of account keys.

Comment 1

[Ninoschka Baca]

Marking critical because we can't test mail in turbo mode. 

Comment 2

[Jaime Rodriguez, Jr.]

This looks pretty bad, if it is still happening ... going ahead and nsbranch+ it
myself for PDT nomination. Request PDT+

Comment 3

[Todd Pringle]

Conrad, any idea what's going on here and if we can fix it quickly?

Comment 4

[Conrad Carlen (not reading bugmail)]

No, there's this and another mail problem which I've had trouble reproducing and
then the turbo perf problem I've been dealing with today.

Comment 5

[Conrad Carlen (not reading bugmail)]

nbaca, this was found on Branch Build 2001-09-06. Using that, I was getting what
you found here:

b. "account1, account2, account3, account1, account2, account3, account4, account5"

On my debug trunk builds over the last few days, I am not getting that. Can you
confirm whether this is still happening for you?

Comment 6

[Todd Pringle]

Conrad, how's this one coming?  This looks like a potential show stopper for
turbo.  Are building on the branch (that's what we're shipping obviously)?  Thanks.

Comment 7

[Todd Pringle]

This should be targeted to 0.9.5 - can you update the milestone Conrad?

Comment 8

[Todd Pringle]

This should be targeted to 0.9.5 - can you update the milestone Conrad?

Comment 9

[Conrad Carlen (not reading bugmail)]

Attachment: patch to fix multiple account keys

Comment 10

[Conrad Carlen (not reading bugmail)]

Found the problem: account keys are stored in a string. When an account is added
it gets concatenated onto mAccountKeyList. Problem was, on UnloadAccounts(), the
string was never being reset so account keys just keep getting added.

Need r=/sr=

Comment 11

[Suresh Duddi (gone)]

Comment on attachment 49790 [details] [diff] [review]
patch to fix multiple account keys

r=dp

Comment 12

[Suresh Duddi (gone)]

mscott, could you super review this one.

Comment 13

[Scott MacGregor]

Comment on attachment 49790 [details] [diff] [review]
patch to fix multiple account keys

sr=mscott

Comment 14

[Conrad Carlen (not reading bugmail)]

Attachment: better patch

Comment 15

[Conrad Carlen (not reading bugmail)]

More cleaning up on shutdown. Fixes both problems reported here.

Comment 16

[Suresh Duddi (gone)]

Comment on attachment 49807 [details] [diff] [review]
better patch

r=dp

Comment 17

[racham]

r=bhuvan

Comment 18

[Conrad Carlen (not reading bugmail)]

*** Bug 100336 has been marked as a duplicate of this bug. ***

Comment 19

[Scott MacGregor]

Comment on attachment 49807 [details] [diff] [review]
better patch

sr=mscott

Comment 20

[Jaime Rodriguez, Jr.]

check it in today - PDT+

Comment 21

[Conrad Carlen (not reading bugmail)]

Checked into branch, trunk next.

Comment 22

[lchiang]

nbaca - let's verify this on the branch when we have builds.  Thanks.

Comment 23

[Conrad Carlen (not reading bugmail)]

Checked into trunk.

Comment 24

[Ninoschka Baca]

Just waiting for the builds now.

Comment 25

[Ninoschka Baca]

Branch build 2001-09-21-05: WinMe

Results so far:

1a. After creating a new account the pref now displays the correct account 
information. 

2. After migrating an account (qatest20), exit/restart, creating a new profile 
and configuring for the same account (qatest20). - It creates the account and no 
longer displays an error but:

a. after logging into the account I can see the messages in the thread pane. 
Select a message but the message pane is BLANK! I tried resizing, tried an 
exit/restart but the message pane remains blank.  

I added another account (qatest33) and I was able to see the contents of its 
messages.

Should I log a new bug for issue "2a"?

Comment 26

[Ninoschka Baca]

Reopening since this doesn't appear to be working as expected.

Comment 27

[Conrad Carlen (not reading bugmail)]

Minusing for branch because this was caused by multi-profile support and profile
startup/shutdown in turbo. That is being turned off in the branch. See bug 101364.

Comment 28

[Jaime Rodriguez, Jr.]

correcting the nsbranch- to show on the keyword field.

Comment 29

[Conrad Carlen (not reading bugmail)]

-> 0.9.6

Comment 30

[Conrad Carlen (not reading bugmail)]

*** Bug 87379 has been marked as a duplicate of this bug. ***

Comment 31

[Conrad Carlen (not reading bugmail)]

Mass move to 0.9.7

Comment 32

[Conrad Carlen (not reading bugmail)]

Ninoschka, the remaining problem with this was the one you gave in comment 25.
While I doubt that's caused by turbo profile changes, I wasn't able to repro it.
Can you try this again with a current trunk build and see what the status is?

Comment 33

[Ninoschka Baca]

I will try reproducing.

Comment 34

[Ninoschka Baca]

Trunk build 2001-12-03-03: WinMe
Now in the new profile a new account does not appear and it crashes after an
exit. Here are the details:

Original Scenarios #2:
1. Migrated a profile (qatest33/nsmail-1), preferences, Advanced to start Quick
Launch
2. Exit
3. Restart the same profile, opened mail and was able to login and view mail
4. Exit
5. Created a new profile, opened mail and configured it for a POP account (I
tried this scenario twice- once for qatest33/nsmail-1 and a second using
qatest20/nsmail-2)

Actual Results: Even though the Account Wizard dialog closes after clicking on
the Finish button, no account appears in the folder pane. Then it crashes.
Unfortunately Talk Back is down.

Comment 35

[Conrad Carlen (not reading bugmail)]

Attachment: console output from this problem

I was able to repro this with a debug build made this morning. I didn't even
have to do the migration step. Here is the console output in case it's obvious
to somebody who knows this code.

Comment 36

[Conrad Carlen (not reading bugmail)]

Comment on attachment 49790 [details] [diff] [review]
patch to fix multiple account keys

Not relevant to current problem.

Comment 37

[Conrad Carlen (not reading bugmail)]

-> 0.9.8

Comment 38

[Conrad Carlen (not reading bugmail)]

Now that bug 116306 is fixed, I'm working on this. After creating an account in
a new profile, nothing appears in the folder pane. But, after exiting, the
account is there.

Comment 39

[racham]

Conrad, Update the tree to make sure that you are not hitting bug
http://bugzilla.mozilla.org/show_bug.cgi?id=119404. If you did update and still
see the problem, then the culprit is some where else.

Comment 40

[Conrad Carlen (not reading bugmail)]

  Bhuvan, unfortunately that wasn't it. On a fresh build, problem still exists.
It still creates the account info properly. After complete quit (though it
crashed on quitting) and restart, the account data is there next time around.
  So, the problem is not in making the account, but between the actual account
data and its display in the XUL document that is the mail window. On quitting,
even in turbo, the mail window was destroyed but, somewhere, some state is being
held between profile switches and causing this.

Comment 41

[Conrad Carlen (not reading bugmail)]

In progress, but not gonna make 0.9.8. -> 0.9.9

Comment 42

[Jaime Rodriguez, Jr.]

nominating

Comment 43

[Peter Trudelle]

nsbeta1+ per ADT triage team

Comment 44

[Conrad Carlen (not reading bugmail)]

Bhuvan gave me the points of interest in the account manager and msg data source
to look at when the account is made. From his debugging and mine, that all
checks out. The next thing checked was the XUL outliner on the premise that the
data is there but it's just not being drawn. The problem is not in the drawing
of the outliner - it doesn't draw anything because it has a row count of zero.

On a successful account creation (where the outliner is not empty afterwards)
done by starting without turbo, with -mail, and making 1 POP account, rows get
added to the outliner in this stack:

nsOutlinerBoxObject::RowCountChanged(nsOutlinerBoxObject * const 0x050da2e0, int
0, int 1) line 391
nsXULOutlinerBuilder::ReplaceMatch(nsIRDFResource * 0x058bdb60, const
nsTemplateMatch * 0x00000000, nsTemplateMatch * 0x05629c70) line 1127
nsXULTemplateBuilder::FireNewlyMatchedRules(const nsClusterKeySet & {...}) line 565
nsXULTemplateBuilder::OnAssert(nsXULTemplateBuilder * const 0x05039cdc,
nsIRDFDataSource * 0x05039480, nsIRDFResource * 0x049f95d0, nsIRDFResource *
0x01663030, nsIRDFNode * 0x058bdb60) line 596
CompositeDataSourceImpl::OnAssert(CompositeDataSourceImpl * const 0x05039484,
nsIRDFDataSource * 0x0519c760, nsIRDFResource * 0x049f95d0, nsIRDFResource *
0x01663030, nsIRDFNode * 0x058bdb60) line 1556
nsMsgRDFDataSource::assertEnumFunc(nsISupports * 0x05039484, void * 0x00129250)
line 402
nsSupportsArray::EnumerateForwards(nsSupportsArray * const 0x0519ca00, int
(nsISupports *, void *)* 0x04b5b020
nsMsgRDFDataSource::assertEnumFunc(nsISupports *, void *), void * 0x00129250)
line 684 + 20 bytes
nsMsgRDFDataSource::NotifyObservers(nsIRDFResource * 0x049f95d0, nsIRDFResource
* 0x01663030, nsIRDFNode * 0x058bdb60, int 1, int 0) line 386
nsMsgAccountManagerDataSource::OnServerLoaded(nsMsgAccountManagerDataSource *
const 0x0519c794, nsIMsgIncomingServer * 0x058bc220) line 1142
nsMsgAccountManager::NotifyServerLoaded(nsMsgAccountManager * const 0x049f6c20,
nsIMsgIncomingServer * 0x058bc220) line 1651

Running turbo, using mail with one profile, closing all windows, starting mail
again, and making 1 POP account, I get the same stack, up to the point of
FindNewlyMatchedRules(). However, in this case, RowCountChanged() is never hit.

Waterson, can you help here?

Comment 45

[Brendan Eich [:brendan]]

Waterson's on sabbatical. Cc'ing tingley in case he can help.

/be

Comment 46

[Chase Tingley]

Sorry, I'm having a bit of trouble following this -- the current problem still
involves creating a new account in turbo mode, right?

I'll see what I can figure out from the code, but since I do all my work on
Linux, I won't have much else to work with.

Comment 47

[(not reading, please use [e-mail] instead)]

I'm on win2k, I can try to help.

Comment 48

[Ninoschka Baca]

Trunk build 2002-02-27: WinMe
In a new profile I configure it for a mail account but don't see the account in 
the folder pane, although the account information is in Account Settings. After 
an exit/restart I still don't see the folders.

Comment 49

[Jan Varga [:janv]]

I can reproduce this bug, but I'm not convinced that it's an outliner problem.

This might help:
- launch JS console when folder pane comes up blank
- see the error
- comment out code in UpgradeFolderPaneUI() - msgMail3PaneWindow.js

function UpgradeFolderPaneUI()                                                   
{
/*
  var folderPaneUIVersion = pref.getIntPref("mail.ui.folderpane.version");     
                                                                  
  if (folderPaneUIVersion == 1) {                               
    var folderUnreadCol = document.getElementById("folderUnreadCol");               
    folderUnreadCol.setAttribute("hidden", "true");                                
    var folderTotalCol = document.getElementById("folderTotalCol");                
    folderTotalCol.setAttribute("hidden", "true");                                 
    pref.setIntPref("mail.ui.folderpane.version", 2);                              
  }
*/
}

So I guess that there is no "mail.ui.folderpane.version" pref for the new profile.

Comment 50

[Conrad Carlen (not reading bugmail)]

> So I guess that there is no "mail.ui.folderpane.version" pref for the new profile.

Yep, and now I see why. Looks like the fix is a one-liner in prefapi.cpp.
Testing that patch...

Comment 51

[Conrad Carlen (not reading bugmail)]

Attachment: patch

This patch fixes the problem of a blank folder pane after exiting in
QuickLaunch, starting again, making a new profile, and then making a new mail
account. Here's why: When the profile switch happens in switching to the new
profile, all user prefs are reset. In this case, the pref
"mail.ui.folderpane.version" had both a default value and a user value. Having
the user value caused it to get nuked on the profile switch. The patch makes it
so, on a profile switch, prefs clears the PREF_USERSET flag on the pref so it
no longer has a user value but its default value is still intact.

Comment 52

[Conrad Carlen (not reading bugmail)]

CC'ing Brian for review. He knows the gory details of this situation.

Comment 53

[Brian Nesse (gone)]

Comment on attachment 75455 [details] [diff] [review]
patch

There is a more thorough fix, but it should wait until after 1.0. This is good
for now. r=bnesse.

Comment 54

[Conrad Carlen (not reading bugmail)]

I added this comment to the code:

+        // Note that we're not unhashing the pref. A pref which has both
+        // a user value and a default value needs to remain. Currently,
+        // there isn't a way to determine that a pref has a default value,
+        // other than comparing its value to BOGUS_DEFAULT_XXX_PREF_VALUE.
+        // This needs to be fixed. If we could positively identify a pref
+        // as not having a set default value here, we could unhash it.
+        
         pref->flags &= ~PREF_USERSET;
         if (gCallbacksEnabled)
             pref_DoCallback(pref->key);
-        return PL_DHASH_REMOVE;
     }
     return PL_DHASH_NEXT;

Comment 55

[Conrad Carlen (not reading bugmail)]

CC'ing Alec for sr=.

Comment 56

[Alec Flett]

Comment on attachment 75455 [details] [diff] [review]
patch

wierd. Can we check CVS blame to see if this was there to fix another bug? I
mean, your fix makes sense, but it seems wierd that it was there...
sr=alecf

Comment 57

[Conrad Carlen (not reading bugmail)]

> Can we check CVS blame to see if this was there to fix another bug?

The PL_DHASH_REMOVE? It was there because bug 96514 was not fixed (or maybe
known). Because of that, if a pref had the user value cleared, it really needed
to be unhashed. Still, I think it *should* be unhashed. 

Comment 58

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 75455 [details] [diff] [review]
patch

a=dbaron for trunk checkin

Comment 59

[Conrad Carlen (not reading bugmail)]

Fix checked in. This won't fix all turbo mailnews problems (see bug 128386) but
fixes the blank folder pane one. Marking resolved for that one. Other problems
have been reported and fixed on this bug. I don't think this needs to stay open
as a sort of META bug for these problems.

Comment 60

[esther]

Using build 20020401 on winxp, I verified original scenario 1 & 2 do not show
the problem as described.  I also verified the scenario in 34, again this
problem did not show up with this build.  So this can be verified for these
problems, however there are still profile mixing going on with this build,
specifically as noted in bug 128386 which is alive and kicking.  I will log
additonal problems as new bugs and verify this one.  If anyone disagrees or sees
any of these scenarios regress, reopen the bug.

Comment 61

[esther]

What a difference a day makes! Comment # 25 2a is back.  I will log a new bug
specifically for it and attach images. Also note: when I first created the
account, the account didn't show up as stated in comment #38 so I will include
that in the new bug.  As far as the Blank Folder pane which was the fix for this
bug, I don't see that it's blank, I see the Local Folders in that pane so this
is still verified.