I don't know of a good way to see which account is the default one (yes, seeing if the "Set as Default" meun item is disabled is a way, but not very good). I suggest showing the default account name in bold (or other form of highlighting) in the Account manager's tree of accounts. This feature was there at least in Thunderbird 2.0. I could find out why it vanished. Either it was intentionally removed or it is a regression. So until a proof of intent is found, I'll mark it a regression. I'd even throw in a tooltip saying what the bold color means.

Here is the patch. It popped out that all the themes still contained the "treechildren::-moz-tree-cell-text(isDefaultServer-true) { font-weight: bold; }" declarations. But no element was ever marked properties="isDefaultServer-true" so that was the regression. The real meat of the patch is the markDefaultServer function. All the rest in the 'function at_build()' is just cleanup (services/JS warning). When another account is made default, the bold highlight moves onto the new server.

Actually it looks like only the pinstripe (Mac?) theme is missing the style for this: http://mxr.mozilla.org/comm-central/search?string=isDefaultServer-true&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central So I can add it there if this is approved.

Comment on attachment 611030 [details] [diff] [review] patch >+function markDefaultServer(oldDefault) { As both callers to this function already have MailServices.accounts, could pass defaultAccount as first argument. function markDefaultServer(defaultAccount, oldDefault) { >+ let accountTree = document.getElementById("account-tree-children"); >+ let defaultAccount = MailServices.accounts.defaultAccount; Then this could then be dropped. >+ let previousDefault = MailServices.accounts.defaultAccount; >+ MailServices.accounts.defaultAccount = currentAccount; >+ markDefaultServer(previousDefault); markDefaultServer(currentAccount, previousDefault); >+ markDefaultServer(); markDefaultServer(mgr.defaultAccount); r=me with that address/answered

Thanks, good optimization.

Sorry I missed that when an account is deleted, the default account can change and needs to be highlighted anew. So this patch updates onDeleteAccount for this.

Comment on attachment 614099 [details] [diff] [review] patch v3 Mac doesn't have the "treechildren::-moz-tree-cell-text(isDefaultServer-true)" rule, so it doesn't show up in bold there… Thanks, Blake.

OK, try this.

Comment on attachment 615856 [details] [diff] [review] patch v4 Looks good to me. We might want to also add a tooltip saying that this is the default account, but even if you didn't want to do that part, this bit gets a ui-r=me. Thanks, Blake.

I wanted a tooltip but I think somebody (Neil?) told me there can't be any on a tree cell element.

(In reply to aceman from comment #9) > I wanted a tooltip but I think somebody (Neil?) told me there can't be any > on a tree cell element. Nothing under the <treechildren> exists from the point of view of layout. (This is assuming you're using a DOM-driven tree in the first place, of course). So you can't attach a tooltip to it. While you can attach a tooltip to the <treechildren>, this will override the default tooltip that shows the whole text for a cropped value.

Comment on attachment 615856 [details] [diff] [review] patch v4 Review of attachment 615856 [details] [diff] [review]: ----------------------------------------------------------------- Before I pass judgement on this, I've noticed that you've only altered accountManage.css for pinstripe. Is it true that we only want this for OSX, and not for Ubuntu or Windows? ::: mailnews/base/prefs/content/AccountManager.js @@ +433,5 @@ > +function markDefaultServer(newDefault, oldDefault) { > + let accountTree = document.getElementById("account-tree-children"); > + for each (let accountNode in accountTree.childNodes) { > + if (newDefault == accountNode._account) { > + accountNode.firstChild.firstChild Let's break this up, like: accountNode.firstChild .firstChild .setAttribute("properties", "isDefaultServer-true"); @@ +437,5 @@ > + accountNode.firstChild.firstChild > + .setAttribute("properties", "isDefaultServer-true"); > + } > + if (oldDefault && oldDefault == accountNode._account) { > + accountNode.firstChild.firstChild Same as above. @@ +469,5 @@ > + let prettyName = server.prettyName; > + > + let canDelete = false; > + if (Components.classes["@mozilla.org/messenger/protocol/info;1?type=" + type] > + .getService(Components.interfaces.nsIMsgProtocolInfo).canDelete) Let's line up the period before getService with the period between Components and classes. If that puts us over 80 chars, we can shorted even more by aliasing, like: let info = Components.classes["@mozilla.org/messenger/protocol/info;1?type" + type] .getService(Components.interfaces.nsIMsgProtocolInfo); if (info.canDelete) canDelete = true; else canDelete = server.canDelete; @@ +478,5 @@ > + if (!canDelete) > + return; > + > + let bundle = document.getElementById("bundle_prefs"); > + let confirmRemoveAccount = Nit - let's try: let confirmRemoveAccount = bundle.getFormattedString("confirmRemoveAccount", [prettyName]);

Ah, I've just noticed that both gnomestripe and qute seem to have that CSS rule covered.

Comment on attachment 615856 [details] [diff] [review] patch v4 Review of attachment 615856 [details] [diff] [review]: ----------------------------------------------------------------- Ok, this seems to work as advertised, and I dig the code-cleanup. With the nits I found fixed, r=me.

(In reply to Mike Conley (:mconley) from comment #12) > Ah, I've just noticed that both gnomestripe and qute seem to have that CSS > rule covered. Yeah, that is the proof this is a regression :) (In reply to Mike Conley (:mconley) from comment #11) > let info = Components.classes["@mozilla.org/messenger/protocol/info;1?type" > + type] > .getService(Components.interfaces.nsIMsgProtocolInfo); > > if (info.canDelete) > canDelete = true; > else > canDelete = server.canDelete; This was previously there, I just wanted to inline it. I can revert it if it is ugly.

Thanks, all fixed. The info-protocol line fix exactly at 80 chars so I didn't need to un-inline it :)

fix=>fits ;)

Tests checked in. https://hg.mozilla.org/comm-central/rev/0cf5688a02e9