100952 - Accounts displayed twice under "Get Msg" dropdown arrow

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P3)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

In recent linux trunk builds (first noticed this in 2001092017 build), clicking
the drop down arrow under "Get Msg" will display each account twice.  They are
displayed only once in the account pane on the left.

Comment 1

[Pratik]

I see this too. but not all accounts are displayed twice. My main account
(default account) was not being displayed twice. 

So I tried changing the default account and guess what? The problem went away.
Accounts got displayed only once (you have to restart Mail/News though). I
reverted back to my previous default and I couldn't reproduce the problem. It
seems changing the default and restarting Mail/News gets rid of the problem.

Comment 2

[laurel]

I saw this on one of my profiles using sep20 trunk build win98.  I had been
changing default accounts on that profile, too.

Comment 3

[R.K.Aa.]

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

Comment 4

[R.K.Aa.]

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

Comment 5

[Greg Onufer]

I see this on Solaris with recent builds, platform should be more
than "PC".

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Comment 7

[Jacek Piskozub]

In my case I see all three accounts twice. Changing the default account (and
restarting Mozilla) did not help.

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Comment 9

[[e-mail]]

This may be a problem with templates that I have been experiencing for some months.
The asynchronous data source appears to be populating the template twice.
The workaround was to add the data source in the onload handler instead of using
the datasources attribute, although I have not tried it in this case.

Comment 10

[[e-mail]]

If you open a message window you will see that the Get Messages button menu does
not show the duplicate messages. This is because this bug is really a duplicate
of bug 73953. There template content generation in menus and menubuttons appears
to get deferred but obviously this doesn't apply to the new toolbaruttons. Since
ideally this should be fixed properly for all widgets, I'm CCing waterson.

Comment 11

[Jacek Piskozub]

In bug 73953 there is a comment that the problem does not occur in the Modern
theme. Well, this one does. Are they really identical?

Comment 12

[Chris Waterson]

I'll take a look.

Comment 13

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

data point:

I have a patch in #96748 that prevents the multiple inboxes from showing up on 
imap accounts when you first run after migration from 4.x

the patch moves the setting of the ref and datasources attributes from the 
onload handler to the xul.

but the the problem is that the next time I start up mail, I get duplcated 
accounts in the folder pane, but the "Get Msg" dropdown is ok (accounts only 
appear once).

Comment 14

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

I think neil is right, we can work around this problem by setting up the ref 
and datasources lazily (or on the onload handler) until #73953 is fixed.

I'll go try to come up with a hack for that, for the 0.9.4 branch.

Comment 15

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

hack fix in hand for the branch.  

similar to the hack for #98418, setting the ref attribute again fixes the 
problem.

Comment 16

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

Attachment: hack fix.

Comment 17

[Chris Waterson]

Sure, r/sr=waterson. It looks like bug 73953 might cover the ``real'' problem,
so if you want to check this in on the branch and the trunk, that's fine with me.

Comment 18

[Scott MacGregor]

Comment on attachment 51898 [details] [diff] [review]
hack fix.

r=mscott
sr=waterson

Comment 19

[Scott MacGregor]

this is the same kind of fix we want to take for 98418 so adding the same
keyword status.

Comment 20

[Scott MacGregor]

actually Chris, do you want to leave this bug open for a real fix or does 73953
cover that already? We can just use 98418 to track this work around and the work
around in 98418 (since they are both similar) if you are going to want to keep
this bug open.

Comment 21

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

http://bugzilla.mozilla.org/show_bug.cgi?id=102978 covers the backout when
#73953 gets fixed.  

when I land my hacks, this bug and #98418 will be fixed.

taking back from waterson, as he owns #73953.

Comment 22

[Chris Waterson]

Attachment: this is a riskier patch, but probably what ought to go on the trunk

Comment 23

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

back to chris.

this is fixed (with a hack) on the branch.

Comment 24

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

is there such a thing as vbranch?  nbaca, this would be something to check on 
the branch only.  for the trunk, that has to wait until waterson lands.

Comment 25

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

Comment on attachment 51944 [details] [diff] [review]
this is a riskier patch, but probably what ought to go on the trunk

r=sspitzer

Comment 26

[Chris Waterson]

Attachment: zap the same problem in the outliner builder, while we're at it.

Comment 27

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

Comment on attachment 51962 [details] [diff] [review]
zap the same problem in the outliner builder, while we're at it.

r=sspitzer

Comment 28

[Ninoschka Baca]

I have not seen this problem on the branch builds. I also recall Laurel saying
that she hasn't seen the problem on the branch.

Comment 29

[David Hyatt]

sr=hyatt

Comment 30

[Scott MacGregor]

I'm removing the nsBranch+ here 'cause Seth's hack fix went into the branch
yesterday for this. So we don't need this for the branch. Let me know if I
misunderstood something here.

Comment 31

[Asa Dotzler [:asa]]

Comment on attachment 51962 [details] [diff] [review]
zap the same problem in the outliner builder, while we're at it.

a=asa (on behalf of drivers) for checkin to 0.9.5.

Comment 32

[Chris Waterson]

Fix checked in.

Comment 33

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

Backed out - reopening.  See bug 103331.

Comment 34

[Chris Waterson]

-> mozilla-0.9.6

Comment 35

[Scott MacGregor]

clearing out an old dependency.

Comment 36

[R.K.Aa.]

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

Comment 37

[R.K.Aa.]

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

Comment 38

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Comment 39

[Jaon]

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

Comment 40

[laurel]

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

Comment 41

[D. Yutzy]

I also have this in build 2001101117 0.9.5 (Windows)...Each account is duped
under Get Msg button in Mail/News

Comment 42

[Lars Linek]

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

Comment 43

[Lars Linek]

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

Comment 44

[Scott Tran (Wildcard)]

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

Comment 45

[Koike Kazuhiko]

Did somebody fix this bug? 2001110103/Win2k works fine.

Comment 46

[Christopher Aillon (sabbatical, not receiving bugmail)]

No.  This is still an issue on linux 2001-11-01-12.

However, if you open Mail/News, close it, and re-open it the problem goes away.

Comment 47

[Schelstraete Bart]

It's maybe stupid, but I installed Netscape 6.2, and in that version the problem
doesn't exist .
I also installed the latest nightly build -> problem still exist on that build.
(win32 version)

Comment 48

[Chris Waterson]

The mail guys hacked a fix into the branch for 6.2, and have been gracious
enough to allow me to try to do ``the real fix'' for the trunk. I'll get this in
for mozilla-0.9.6, I swear.

Comment 49

[Gilles Durys]

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

Comment 50

[laurel]

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

Comment 51

[Chris Waterson]

Attachment: maintain a stack of ``activated'' resources that we're currently building for

Comment 52

[Chris Waterson]

My original patch caused regressions because we'd maintain a single binary ``are
we building'' state in the builder. But the builder may re-enter itself when
it's creating content for a container (i.e., via a request to the content model
on newly appended content).

The way to fix this is to maintain a stack in the template builder of
``activated'' resources for which we're currently building content, and to only
bail when we detect that we're about to build content for the _same_ resource.

Comment 53

[Chris Waterson]

Okay, let me re-summarize what's going on here. This bug happens because we get
an OnAssert while we're building the content model for the drop-down button
(specifically, one of our GetTargets calls causes something in the mailnews
datasource to bootstrap itself, and it notifies us in a re-entrant way). This
causes us to ``double up'' the content, because we build it once while
processing the OnAssert, and then build it again when we unwind out from the
GetTargets.

We have a heavy-handed mechanism for ignoring assertions while we're building
content that was meant to get around this exact problem; however, I applied it
incorrectly (and caused many regressions). Specifically, by stifling any
recursive content construction from CreateContents, I bungled the case where
we're CreateContents-ing, and the somebody eagerly asks for more content in that
same subtree. (For example, we're creating contents ``noisly'' and the frame
constructor wants content children.)

The fix I'm proposing here is to make the re-entrancy check specific to the
resources for which we're actively building content. We maintain an ``activation
stack'' of resources in the template builder: CreateContents pushes the resource
onto the stack that it's currently building for, and pops it when its done.
Other entry points that could trigger content construction check the stack and
bail if they see that the resource is already being processed.

We ought to have an analogous mechanism in the outliner builder, but I'll be a
lazy SOB and wait for a bug to present itself. (And am also a bit gunshy given

Comment 54

[Chase Tingley]

Comment on attachment 56793 [details] [diff] [review]
maintain a stack of ``activated'' resources that we're currently building for

Looks good; testing shows no signs of past unpleasantness.  
r=tingley

Comment 55

[Mike Shaver (:shaver emeritus)]

Comment on attachment 56793 [details] [diff] [review]
maintain a stack of ``activated'' resources that we're currently building for

Why do you need an nsAutoVoidArray here?  Why not just use the
C-stack-allocated ActivationEntry, which would then grow an
"up" pointer, and then use an |ActivationEntry *bottom| in your
builder?  Seems like it'd be cleaner to me, and probably cheaper
to boot.

Comment 56

[Chris Waterson]

Attachment: who's been writing fat mozilla code for too long?

Comment 57

[Chase Tingley]

Comment on attachment 56849 [details] [diff] [review]
who's been writing fat mozilla code for too long?

this is neat. r=tingley

Comment 58

[Brendan Eich [:brendan]]

Comment on attachment 56849 [details] [diff] [review]
who's been writing fat mozilla code for too long?

Channeling shaver: sr=[e-mail].

Mmmm, auto-helper-class and double-indirection.

/be

Comment 59

[Chris Waterson]

Crossing fingers and checking in.

Comment 60

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Comment 61

[Mike Shaver (:shaver emeritus)]

Well-channelled.

Comment 62

[Ninoschka Baca]

Trunk build 2001-11-15: WinMe, Linux RH 7.1, Mac X
Marking Verified since it is working for me. Please reopen if you see the 
problem continue.