327473 - [TB/SM] Account Settings not accessible if more then one account

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Thomas Schade]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060215 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060215 Firefox/1.6a1

In my profile I've several POP3 accounts plus several News accounts. Since
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060215 Thunderbird/1.6a1
it is not any longer possible to access the settings for the several accounts but the default account (see screenshot in attachment). It still does not work in todays's nightly: 2006021602

Reproducible: Always

Steps to Reproduce:
1. Have a profile with more than one account (e.g. one POP3 account and one News account)
2. Open 'Tools - Account Settings'
3. Try to change settings for a non-default account

Actual Results:  
Only the default account can be accessed (marked by the [+] symbol).

Expected Results:  
For the other accounts the [+] is missing, account data cannot be accessed.

This has also been reported for Linux builds and also for Seamonkey:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060215 SeaMonkey/1.5a

Comment 1

[Thomas Schade]

Attachment: Screen shot of 'Account Settings', showing missing [+] (21 kB)

Comment 2

[David :Bienvenu]

confirming - I see this too, in my debug trunk build. None of the sub-categories for the non-default servers show up at all, under their respective servers. The 1.8.1 branch is OK, so it's most likely not the favorite folders stuff, which was my initial fear...

Comment 3

[David :Bienvenu]

It's not the default account that's special - I'm not quite sure what's special about the account that does have settings available; it's the second account listed, and not the default. And the Local Folders account has one sub-category displayed for disk space. Weird.

Comment 4

[Thomas Schade]

(In reply to comment #3)
> It's not the default account that's special - I'm not quite sure what's special
> about the account that does have settings available; it's the second account
> listed, and not the default.

It might be just by chance that in my case it's the default account. However, as you can see in the screenshot from my attachment, here it's the first account listed.

> And the Local Folders account has one sub-category
> displayed for disk space. Weird.

Same here, but the respective account here is the first News account (I don't have local folders account).

Comment 5

[Tobias Fischer]

This is not only Thunderbird, SeaMonkey Trunk is also affected almost on Windows and Linux Plattforms. 
I will gve an link to the Sreenshot from yesterdays CREATURE-Tinderbox-Build 2006021504: http://img110.imageshack.us/img110/2160/accountsettings20060215043jw.png
And it seems to bee really weired which accounts are affected, in my case only one of IMAP-Accounts was o.k. 

Comment 6

[Neil Deakin]

The patch in bug 326712 fixes this for me. It contains a fix for some container related issues.

Comment 7

[David :Bienvenu]

thx, yes, the patch in bug 326712 fixes this for me as well. It doesn't fix the assertions on startup, however, bug 327508. It's up to you if you want to dup this bug, or just mark it fixed when you checkin the patch for bug 326712.

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

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

Comment 9

[Edward]

Confirming my bug (327681) as a duplicate of this.  Thanks to Stephen for catching it.

I have noticed this problem did not occur with the 2/16 and 2/17 builds on another system, when an earlier build of SeaMonkey that did not have this problem, had been installed.  It simply used the preferences that were created by the earlier, non-problematic build.

Comment 10

[Edward]

I am retracting the second portion of Comment 9.  

I just checked the settings on the other computer with the 2006021701 Linux nightly SeaMonkey and the same has indeed occurred, account settings only show for one account.

Comment 11

[OstGote!]

This is a Core bug, but I don't have a clue which component fits the best except XP Toolkit (which is difficult to find if you have a problem with Mailnews/TB).

Comment 12

[Magnus Melin [:mkmelin]]

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

Comment 13

[Thomas Schade]

'FIXED' confirmed for:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060224 SeaMonkey/1.5a Build ID: 2006022406
Confirmation not possible for Thunderbird trunk due to the ongoing unavailability of /patrocles/.

Comment 14

[David :Bienvenu]

I built with those changes and it was fixed for me. Marking fixed.

Comment 15

[Thomas Schade]

With today's trunk builds the bug is back again. This applies to:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060228 Thunderbird/1.6a1 ID:2006022802
as well as to:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060228 SeaMonkey/1.5a (2006022807)

Comment 16

[David :Bienvenu]

yes, my trunk build pulled yesterday has the problem again (not quite identical in terms of what accounts are displayed and how, but close enough)

Comment 17

[Thomas Schade]

(In reply to comment #16)
> yes, my trunk build pulled yesterday has the problem again (not quite identical
> in terms of what accounts are displayed and how, but close enough)

Biggest difference to the 'original' bug is, apart from a different account still accessible, that this time the [+] (Expand) is still there. Even more, it switches to [-] when pressed and also back to [+]. But it does not expand (open?) the account data.

Comment 18

[Neil Deakin]

Looks to be a regression from bug 328496

Comment 19

[Neil Deakin]

OK, the problem here is that the account settings list expects there to be several rows in the list with the same resource uri. I could add back support for this, although it will take a few days to implement and test, and would add a bit more overhead.

I notice though that the account list is only using it to display the settings panel labels. Is it possible to use unique panel resources based on the account?

Comment 20

[David :Bienvenu]

possibly - I'm not familiar with that code so I'd have to look at it.

Comment 21

[David :Bienvenu]

Neil, can you say more about what I would change? We use the account manager ds in other places, so we wouldn't want the settings resources to be children of the accounts in the account manager ds...isn't it the containment property of the tree that populates the various settings labels in the left pane? That list is dynamic based on the server, and possible extensions, if you look at mailnews\base\src\nsMsgAccountManagerDS.cpp

Comment 22

[Neil Deakin]

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

Comment 23

[Olivier Vit (just a reporter)]

Attachment: Screen shot in build Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.9a1) Gecko/20060228 SeaMonkey/1.5a

Comment 24

[Olivier Vit (just a reporter)]

Log from the javascript console in Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.9a1) Gecko/20060228 SeaMonkey/1.5a

Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\fullsoft.dll
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\master.ini
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\myspell\en-US.aff
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\myspell\en-US.dic
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\myspell\fr-FR.aff
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\myspell\fr-FR.dic
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\myspell\README-fr-FR.txt
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\talkback-l10n.ini
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\talkback.cnt
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\talkback.exe
Failed to load XPCOM component: D:\Program Files\mozilla.org\SeaMonkey\components\talkback.hlp
Warning:Error in parsing value for property 'padding'. Declaration dropped.
Source File:  chrome://searchstatus/skin/searchstatus.cssLine:  164
Error:  lastItem has no properties
Source File:  chrome://messenger/content/AccountManager.js Line:  198
Warning:Key event not available on GTK2: key="f" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="a" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="c" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on some keyboard layouts: key="f" modifiers="control,alt"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="f" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="a" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="c" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on some keyboard layouts: key="f" modifiers="control,alt"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="f" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="a" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="c" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on some keyboard layouts: key="f" modifiers="control,alt"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="d" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Error:  this.mCurrentBrowser has no properties
Source File:  chrome://global/content/bindings/tabbrowser.xmlLine: 0
Error:  this.mCurrentBrowser has no properties
Source File:  chrome://global/content/bindings/tabbrowser.xmlLine: 0
Warning:Key event not available on GTK2: key="f" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="a" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on GTK2: key="c" modifiers="accel,shift"
Source File:  chrome://navigator/content/navigator.xulLine:  0
Warning:Key event not available on some keyboard layouts: key="f" modifiers="control,alt"

Comment 25

[Neil Deakin]

After much thought, I've found a way of implementing what is needed in the template builder that doesn't affect memory usage or performance, so I'll have a patch for this soon.

Comment 26

[Neil Deakin]

Attachment: allow multiple results with the same id to be used

This should fix the account settings window.

Comment 27

[Boris Zbarsky [:bzbarsky]]

Neil, could you explain what this is actually doing?  Like which parts of the code are doing what?  This is a reasonably big patch, and I'm already swamped with such, so I'll need some help to do this review in any sort of reasonable (next week) timeframe.

Also, given that vlad reviewed the original landing, maybe he can help?

Comment 28

[Neil Deakin]

Sure I can provide a patch with more comments tomorrow. The general idea is to support displaying the same node in multiple places, as in the Account Settings window where 'Server Settings' appears several times, once for each account. nsTemplateMatch is modified to include a field aContainer which is the parent where the item is inserted. This way, a result can be uniquely identified by the container and the child result, allowing the same child id to appear in multiple containers. Places where the list of matches are being iterated over such as in UpdateResult are modified to ensure that the container is the same. The MayGenerateResult function is replaced with a GetInsertionLocations function since a result may be inserted into multiple locations. UpdateResult then iterates over each one.

The other changes just change the fields in nsTemplateMatch so as to not increase the size of the class now that it has the aContainer field. I changed it to use 2 byte indicies of the query and rule instead of pointers. That could have been another bug though.

It doesn't really matter who reviews it, so if you like, feel free to change it.

Comment 29

[Mr David B Weintraub]

If I bring up Mozilla 1.7.x, I can see the settings for the email account. In Seamonkey(Build 2006022009) the settings aren't visable. 

Comment 30

[Neil Deakin]

Attachment: version with more comments

I didn't add many comments, as although the patch looks big, it is mostly changes to argument and field names

bz, did you want me to get vlad to review this instead?

Also, it would be good if some mail folks tested this too.

Comment 31

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 213897 [details] [diff] [review]
version with more comments

Thanks for the explanation!

>Index: content/xul/templates/src/nsTemplateMatch.cpp
>+    // assign the rule, used to indicate that a match is active, and

s/rule/rule index/ right?

>Index: content/xul/templates/src/nsTemplateMatch.h
>+    PRBool IsActive() {
>+        return mRuleIndex >= 0; }

Put that closing '}' on its own line?

>+    // indicate that a rule is no longer active, used when a query with a

Semicolon or period there, not comma.

>+    void SetInactive() {
>+        mRuleIndex = -1; }

Again, '}' on own line.

>+     * The container the content generated for the match is inside.

"The container inside which the content generated for this match lives."

Should mQuerySetPriority really be public?  Should anyone but the constructor be able to write to it?  If the answers are yes and no respectively, perhaps it should just be const?  If no and no, then just make it private and add an accessor?

Similar for mContainer, mRuleIndex, etc.

>Index: content/xul/templates/src/nsTemplateRule.h
>+#define MAX_QUERYRULES 32767

PR_INT16_MAX, please, instead of hardcoding the number?

>Index: content/xul/templates/src/nsXULContentBuilder.cpp
>+                          nsISupportsArray** aLocations);

Hmm...  This is nsISupportsArray because of what GetElementsForID returns?  If so, comment that here, along with the bug number of the bug on switching both of these over to nsCOMArray<nsIContent>& ?

>@@ -1294,63 +1294,81 @@ nsXULContentBuilder::CreateContainerCont

>+            rv = DetermineMatchedRule(aElement, nextresult, aQuerySet,
>+                                      &matchedrule, ruleindex);

I'd really prefer we use PRInt16* for the out param; makes it clearer what's going on.

>+        if (removematch) {
>+            newmatch->mNext = removematch->mNext;
>             nsTemplateMatch::Destroy(mPool, existingmatch);

Do you want to destroy removematch here?  Or existingmatch?  Or are they guaranteed equal?  In the last case, assert so.

>+nsXULContentBuilder::GetInsertionLocations(nsIXULTemplateResult* 

>+        // clear the item in the list since we don't want to insert there
>+        elements->SetElementAt(t, nsnull);

I assume the idea of not removing the element is that this is faster?  Ok if so.

>+    *aLocations = elements;
>+    NS_ADDREF(*aLocations);

How about:

  elements.swap(*aLocations);

instead of those two lines?

I'm up to the beginning of nsXULTemplateBuilder.cpp; more comments later today, I hope.

Comment 32

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 213897 [details] [diff] [review]
version with more comments

>Index: content/xul/templates/src/nsXULTemplateBuilder.cpp
>+    // just remove the old result and then add a new result separately

So given that, does it still make sense to have UpdateResult be a single method?  If not, file a followup bug to split it into RemoveResult and AddResult?  Your call, in any case.

>Index: content/xul/templates/src/nsXULTreeBuilder.cpp
> nsXULTreeBuilder::GetTemplateActionRowFor(PRInt32 aRow, nsIContent** aResult)
>+    // The match stores the indicies of the rule and 

"indices"

r+sr=bzbarsky with those nits picked.

Comment 33

[Andrew Schultz]

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

Comment 34

[Neil Deakin]

Checked in. Will revisit the array usage in bug 321174

Comment 35

[Mr David B Weintraub]

In addition to not showing the setttings. The software doesn't do a automatic conection to the server and download new email, this must be done manually.

Please marke this bug as unresolved or move to Seamonkey.

Comment 36

[Hartmut Figge]

(In reply to comment #35)
> In addition to not showing the setttings. The software doesn't do a automatic
> conection to the server and download new email, this must be done manually.

The checkin has fixed the problem for my self compiled SM on linux. Mails and News are automatically fetched.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060306 SeaMonkey/1.5a Mnenhy/0.7.3.10002

Comment 37

[Andrew Schultz]

David: that's a completely different bug.  If you can't find an existing bug that describes your problem, you should file a new bug.

Comment 38

[David :Bienvenu]

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

Comment 39

[Mr David B Weintraub]

updated SeaMonkey to Build ID: 2006030909 and now all mail accounts have their settings accessable.

Comment 40

[Magnus Melin [:mkmelin]]

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