MailNews problems using Turbo Mode

VERIFIED FIXED in mozilla1.0

Status

--
critical
VERIFIED FIXED
18 years ago
5 years ago

People

(Reporter: nbaca, Assigned: ccarlen)

Tracking

Trunk
mozilla1.0
x86
Windows ME
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

18 years ago
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.
(Reporter)

Comment 1

18 years ago
Marking critical because we can't test mail in turbo mode. 
Severity: normal → critical
Keywords: mailtrack, nsbranch
QA Contact: sairuh → jrgm
This looks pretty bad, if it is still happening ... going ahead and nsbranch+ it
myself for PDT nomination. Request PDT+
Keywords: nsbranch → nsbranch+
QA Contact: jrgm → tpreston

Comment 3

18 years ago
Conrad, any idea what's going on here and if we can fix it quickly?
(Assignee)

Comment 4

18 years ago
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.

Updated

18 years ago
Blocks: 99488
(Assignee)

Comment 5

18 years ago
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?

Updated

18 years ago
Blocks: 75599

Updated

18 years ago
No longer blocks: 75599

Comment 6

18 years ago
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

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

Comment 8

18 years ago
This should be targeted to 0.9.5 - can you update the milestone Conrad?
(Assignee)

Comment 9

18 years ago
Created attachment 49790 [details] [diff] [review]
patch to fix multiple account keys
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=
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.5
Comment on attachment 49790 [details] [diff] [review]
patch to fix multiple account keys

r=dp
Attachment #49790 - Flags: review+
mscott, could you super review this one.

Comment 13

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

sr=mscott
Attachment #49790 - Flags: superreview+
More cleaning up on shutdown. Fixes both problems reported here.
Comment on attachment 49807 [details] [diff] [review]
better patch

r=dp
Attachment #49807 - Flags: review+

Comment 17

18 years ago
r=bhuvan
*** Bug 100336 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Blocks: 75599

Comment 19

18 years ago
Comment on attachment 49807 [details] [diff] [review]
better patch

sr=mscott
Attachment #49807 - Flags: superreview+
check it in today - PDT+
Whiteboard: PDT+
Checked into branch, trunk next.

Comment 22

18 years ago
nbaca - let's verify this on the branch when we have builds.  Thanks.
QA Contact: tpreston → nbaca
Whiteboard: PDT+ → PDT+ [FIXED ON BRANCH]
Checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Whiteboard: PDT+ [FIXED ON BRANCH] → PDT+ [FIXED ON BRANCH & TRUNK]
(Reporter)

Comment 24

18 years ago
Just waiting for the builds now.
(Reporter)

Comment 25

18 years ago
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"?
(Reporter)

Comment 26

18 years ago
Reopening since this doesn't appear to be working as expected.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Whiteboard: PDT+ [FIXED ON BRANCH & TRUNK] → nsbranch-
correcting the nsbranch- to show on the keyword field.
Keywords: nsbranch+ → nsbranch-
Whiteboard: nsbranch-
-> 0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6
*** Bug 87379 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Blocks: 107067

Updated

17 years ago
Keywords: nsbranch-
Mass move to 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Updated

17 years ago
Blocks: 108795
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?
(Reporter)

Comment 33

17 years ago
I will try reproducing.
(Reporter)

Comment 34

17 years ago
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.
Created attachment 60320 [details]
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 on attachment 49790 [details] [diff] [review]
patch to fix multiple account keys

Not relevant to current problem.
Attachment #49790 - Attachment is obsolete: true
-> 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
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

17 years ago
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.
  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.
In progress, but not gonna make 0.9.8. -> 0.9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9
nominating
Keywords: nsbeta1

Updated

17 years ago
No longer blocks: 107067

Comment 43

17 years ago
nsbeta1+ per ADT triage team
Keywords: nsbeta1 → nsbeta1+
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?
Waterson's on sabbatical. Cc'ing tingley in case he can help.

/be

Comment 46

17 years ago
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.
I'm on win2k, I can try to help.
(Reporter)

Comment 48

17 years ago
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.
(Assignee)

Updated

17 years ago
Target Milestone: mozilla0.9.9 → mozilla1.0
(Assignee)

Updated

17 years ago
Depends on: 128386

Comment 49

17 years ago
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.
> 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...
Created attachment 75455 [details] [diff] [review]
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.
(Assignee)

Updated

17 years ago
Attachment #49807 - Attachment is obsolete: true
Attachment #60320 - Attachment is obsolete: true
CC'ing Brian for review. He knows the gory details of this situation.

Updated

17 years ago
Attachment #75455 - Flags: review+
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.
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;
CC'ing Alec for sr=.
Status: REOPENED → ASSIGNED

Comment 56

17 years ago
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
Attachment #75455 - Flags: superreview+
> 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. 
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago17 years ago
Resolution: --- → FIXED

Comment 60

17 years ago
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.
Status: RESOLVED → VERIFIED

Comment 61

17 years ago
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. 

Updated

17 years ago
No longer blocks: 75599

Updated

17 years ago
Blocks: 75599
No longer blocks: 99488

Updated

10 years ago
Component: Cmd-line Features → Cmd-line Features
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.