Closed
Bug 813929
Opened 12 years ago
Closed 10 years ago
Lost the SMTP parameters line in Thunderbird
Categories
(MailNews Core :: Account Manager, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 29.0
People
(Reporter: pub, Assigned: aceman)
References
Details
(Keywords: reproducible)
Attachments
(2 files, 7 obsolete files)
115.26 KB,
image/jpeg
|
Details | |
11.15 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/20100101 Firefox/16.0 Build ID: 20121024073032 Steps to reproduce: Nothing. I just try to attempt the SMTP configuration. Actual results: The SMTP configuration line in account parameters windows as disappear. Instead of, there is a blank line. When I click on this line, on the right side of the windows, A new view of TB appears.
Reporter | ||
Updated•12 years ago
|
Reporter | ||
Comment 1•12 years ago
|
||
And the error lines in tools Horodatage : 06/11/2012 16:33:00 Erreur : NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgFolder.prettyName] Fichier Source : chrome://messenger/content/AccountManager.js Ligne : 1185 Horodatage : 06/11/2012 16:32:50 Erreur : SyntaxError: syntax error Fichier Source : chrome://messenger/content/selectionsummaries.js Ligne : 291, Colonne : 24 Code Source : let msgContents = <div class="row"> the end :
Very strange. Can you try in TB safe mode: help -> restart with addons disabled?
Comment 4•12 years ago
|
||
just wanted to add that the user has posted its question on different French forums (including Geckozone) and on MozillaZine without anybody able to help... See: http://forums.mozillazine.org/viewtopic.php?f=39&t=2607725 http://forums.mozfr.org/viewtopic.php?f=4&t=109139 I had never seen that before.
This could be an addon problem or gecko problem (like painting old HW accelerated data).
Comment 6•12 years ago
|
||
Does removing localstore.rdf in your profile helps ?
Reporter | ||
Comment 7•12 years ago
|
||
(In reply to :aceman from comment #2) > Very strange. Can you try in TB safe mode: help -> restart with addons > disabled? Done. Same effect.
Reporter | ||
Comment 8•12 years ago
|
||
(In reply to Ludovic Hirlimann [:Usul] from comment #6) > Does removing localstore.rdf in your profile helps ? Can you explain more of this way. What dou you mean ?
Reporter | ||
Comment 9•12 years ago
|
||
(In reply to :aceman from comment #5) > This could be an addon problem or gecko problem (like painting old HW > accelerated data). If a addon is the problem, it is definitive. Le safe mode as the same effect.
Comment 10•12 years ago
|
||
(In reply to Pub@salard.name from comment #8) > (In reply to Ludovic Hirlimann [:Usul] from comment #6) > > Does removing localstore.rdf in your profile helps ? > > Can you explain more of this way. What dou you mean ? Ludovic demande de supprimer le fichier localstore.rdf présent dans ton profil Thunderbird (consulte cet article pour plus de détails sur le profil: https://support.mozillamessaging.com/fr/kb/profil-utilisateur ) Je pense qu'il est préférable d'effectuer cette manipulation en ayant au préalable fermé Thunderbird
Reporter | ||
Comment 11•12 years ago
|
||
(In reply to Ludovic Hirlimann [:Usul] from comment #6) > Does removing localstore.rdf in your profile helps ? No, it does'nt.
Comment 12•12 years ago
|
||
(In reply to :aceman from comment #3) > Wayne, have you ever seen this strangeness? nope. well, not for a long time.
Comment 13•11 years ago
|
||
(priority is reserved for developers https://bugzilla.mozilla.org/page.cgi?id=fields.html#priority )
Priority: P1 → --
Assignee | ||
Comment 14•11 years ago
|
||
Is this a permanent problem? At each open of the account manager and click on SMTP line? What about TB 17.0.5?
Reporter | ||
Comment 15•11 years ago
|
||
(In reply to :aceman from comment #14) > Is this a permanent problem? At each open of the account manager and click > on SMTP line? > > What about TB 17.0.5? The problem is permanent. TB 17.0.5 as the same problem. I think that some files have been corrupted at one time, and now, nothing can make the situation back.
Assignee | ||
Comment 16•11 years ago
|
||
Have you tried downloading the full TB installer for TB 17 ? Do did you just update ?
Reporter | ||
Comment 17•11 years ago
|
||
(In reply to :aceman from comment #16) > Have you tried downloading the full TB installer for TB 17 ? Do did you just > update ? I tried to install the full version of TB 17. Nothing changes. The only thing I have done is to create a new profil. In this case, the problem diseppears.
Assignee | ||
Comment 18•11 years ago
|
||
(In reply to Pub@salard.name from comment #1) > And the error lines in tools > > Horodatage : 06/11/2012 16:33:00 > Erreur : NS_ERROR_FAILURE: Component returned failure code: 0x80004005 > (NS_ERROR_FAILURE) [nsIMsgFolder.prettyName] > Fichier Source : chrome://messenger/content/AccountManager.js > Ligne : 1185 Can you update this error message? In TB 17, the line number could be different.
Reporter | ||
Comment 19•11 years ago
|
||
(In reply to :aceman from comment #18) > (In reply to Pub@salard.name from comment #1) > > And the error lines in tools > > > > Horodatage : 06/11/2012 16:33:00 > > Erreur : NS_ERROR_FAILURE: Component returned failure code: 0x80004005 > > (NS_ERROR_FAILURE) [nsIMsgFolder.prettyName] > > Fichier Source : chrome://messenger/content/AccountManager.js > > Ligne : 1185 > > Can you update this error message? In TB 17, the line number could be > different. I Don't understand what you want. The only error I get is this one : Horodatage : 18/04/2013 20:50:36 Avertissement : La boîte XUL pour l'élément _moz_generated_content_before contient un enfant #text intégré, forçant tous ses enfants à être englobés dans un bloc. Fichier Source : chrome://global/content/bindings/toolbar.xml Ligne : 276
Assignee | ||
Comment 20•11 years ago
|
||
You posted that error message in comment 1 with TB 16. Do you no longer get that error?
Reporter | ||
Comment 21•11 years ago
|
||
(In reply to :aceman from comment #20) > You posted that error message in comment 1 with TB 16. Do you no longer get > that error? I just clear the history. The only error I get is when I try to attempt to the smtp configuration.
Reporter | ||
Comment 22•11 years ago
|
||
(In reply to Pub@salard.name from comment #21) > (In reply to :aceman from comment #20) > > You posted that error message in comment 1 with TB 16. Do you no longer get > > that error? > > I just clear the history. The only error I get is when I try to attempt to > the smtp configuration. I am sorry. I have two errors but they append not when I try to attempt the smtp configuration. Horodatage : 18/04/2013 21:11:45 Erreur : NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgFolder.abbreviatedName] Fichier Source : chrome://messenger/content/folderPane.js Ligne : 1944 and Horodatage : 18/04/2013 21:11:45 Erreur : [Exception... "Component returned failure code: 0x80550007 [nsIMsgFolder.getStringProperty]" nsresult: "0x80550007 (<unknown>)" location: "JS frame :: chrome://messenger/content/folderPane.js :: getSmartFolderName :: line 2436" data: no] Fichier Source : chrome://messenger/content/folderPane.js Ligne : 2438
Assignee | ||
Comment 23•11 years ago
|
||
Pub, have you tried to set up a Instant messaging account in the past?
Assignee | ||
Comment 24•11 years ago
|
||
Seems to be reproducible by Dagger by having a somehow broken IM account from the past.
Assignee: nobody → acelists
Status: UNCONFIRMED → NEW
Ever confirmed: true
Product: Thunderbird → MailNews Core
Assignee | ||
Comment 25•11 years ago
|
||
OK, this is a WIP patch. Dagger, what do you think? Does it make AM in your profile work and still make you aware of the broken accounts?
Attachment #758278 -
Flags: feedback?(dagger.bugzilla)
Comment 26•11 years ago
|
||
Yeah, that does do the trick. SMTP settings are back. The invalid account shows as "Invalid account null"; if I click it, the right hand side of the dialog is blank. Account Actions -> Remove Account is disabled, so I can't delete the account, but at least it's not breaking AM any more. I figured out how to reproduce this. Create a chat account, then hunt down the corresponding messenger.account.accountN.prpl pref, set it to some unrecognized protocol, and restart Tb. Note that if you have one of these broken chat accounts the UI for chat also ends up broken, so we do need to fix the actual bug too, not just cover over it in AM. It might be easiest to just do that (though having AM be resilient to this sort of failure would of course be nice.) The cause of the breakage is here: https://mxr.mozilla.org/comm-central/source/chat/components/src/imAccounts.js#131 i.e. for accounts with unknown protocols, fall back to UnknownProtocol -- the problem is that this code block returns, but the account is only added to the internal list of accounts in the call to gAccountsService._keepAccount(), which is just after the return. Florian suggested fixing it by moving the _keepAccount() call further up the function. Since it depends only on this.numericId, it can be moved up to just after where that's defined.
Assignee | ||
Comment 27•11 years ago
|
||
Thanks for the test. The reasoning of this "papering over" is that even if the rest of the UI is broken we would like at least the AM to be robust and allow e.g. deletion of the broken account. The cause of the broken account may be unknown and some other reason than what you have in your profile. I'll check why deletion is disabled for you, I'd like it enabled. The code tries to show account name or at least key for the "Invalid account" but yours is so broken that it shows neither of them so the result is "Invalid account null". Actually I can fix this, by fetching account.key first which should succeed with higher probability.
Assignee | ||
Comment 28•11 years ago
|
||
Comment on attachment 758278 [details] [diff] [review] WIP patch IanN, do you agree with the way I am going forward here?
Attachment #758278 -
Flags: feedback?(iann_bugzilla)
Reporter | ||
Comment 29•11 years ago
|
||
Sorry for my English, but I have not understood all you said. Do I have something to do ?
Assignee | ||
Comment 30•11 years ago
|
||
Pub, look into file prefs.js and see if there are any prefs like "messenger.accounts" .
Reporter | ||
Comment 31•11 years ago
|
||
It didn't find a occurence of the item messenger.accounts.
Comment 32•11 years ago
|
||
Comment on attachment 758278 [details] [diff] [review] WIP patch (Dagger already gave feedback…)
Attachment #758278 -
Flags: feedback?(dagger.bugzilla)
Attachment #758278 -
Flags: feedback?(mkmelin+mozilla)
Reporter | ||
Comment 33•11 years ago
|
||
Do I have something to do ?
Assignee | ||
Comment 34•11 years ago
|
||
Let TB update to 24 (it is already released). Please comment if the problem is still there. Paste the error from console again (it may have a different line number now).
Reporter | ||
Comment 35•11 years ago
|
||
Horodatage : 20/09/2013 13:33:26 Erreur : NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMsgFolder.prettyName] Fichier Source : chrome://messenger/content/AccountManager.js Ligne : 1192
Assignee | ||
Comment 36•11 years ago
|
||
OK. I think you have not yet replied to comment 23 whether you have some Instant messaging (IM/chat) accounts set up, or whether you have tried to have some in the past on this TB profile/installation.
Attachment #758278 -
Flags: feedback?(mconley)
Reporter | ||
Comment 37•11 years ago
|
||
No, I don't have a instant messaging account neither I tried to have one.
Assignee | ||
Comment 38•11 years ago
|
||
Other than this problem in account manager, all your accounts in Thunderbird are working fine and receiving messages? So I assume you can not apply the patch yourself and build Thunderbird. We will make you a special build for Windows to try running with the patch. It should output some more info to the Error console to help you determine which account is causing the problem. We can then look at that one more closely.
Reporter | ||
Comment 39•11 years ago
|
||
I 'am sorry. For the answer of the 35, it was with TB 17.08. With 24, I don't have any error number but the bug is always present.
Assignee | ||
Comment 40•11 years ago
|
||
Aryx, could you please make a try-build with this patch for Windows (64bit)?
Comment 41•11 years ago
|
||
Pushed to Thunderbird-Try as https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=bc042bb9d673
Comment 42•11 years ago
|
||
Re-pushed because Thunderbird-Try doesn't support win64 at the moment: https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=247748b4b400
Comment 43•11 years ago
|
||
(In reply to dagger.bugzilla from comment #26) > The cause of the breakage is here: > > https://mxr.mozilla.org/comm-central/source/chat/components/src/imAccounts. > js#131 > > i.e. for accounts with unknown protocols, fall back to UnknownProtocol -- > the problem is that this code block returns, but the account is only added > to the internal list of accounts in the call to > gAccountsService._keepAccount(), which is just after the return. Florian > suggested fixing it by moving the _keepAccount() call further up the > function. Since it depends only on this.numericId, it can be moved up to > just after where that's defined. This should probably get filed separately. I'm also Cc'ing clokep who's probably interested in knowing about this bug.
Comment 44•11 years ago
|
||
It was done in bug 861454. (That bug fixed the exception from the chat code I was seeing; the patch in this bug is about making AM handle exceptions in general.)
Comment 45•11 years ago
|
||
Comment on attachment 758278 [details] [diff] [review] WIP patch Review of attachment 758278 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/base/prefs/content/AccountManager.js @@ +1447,5 @@ > + for (let account of accounts) { > + let accountName = null; > + let server = null; > + let accountKey = null; > + let AMChrome = null; lowercase am @@ +1451,5 @@ > + let AMChrome = null; > + let panelsToKeep = []; > + let accountCorrect = true; > + > + try { + need indenting but what are you expecting will throw? better just put *that* part in the try-catch
Attachment #758278 -
Flags: feedback?(mkmelin+mozilla) → feedback+
Assignee | ||
Comment 46•11 years ago
|
||
The point is we do not know what may throw again. So I try the whole block and then we may analyze the exceptions if any are found in the future.
Comment 47•11 years ago
|
||
Yuck:/
Comment 48•11 years ago
|
||
Comment on attachment 758278 [details] [diff] [review] WIP patch Review of attachment 758278 [details] [diff] [review]: ----------------------------------------------------------------- Needs some polish in here, but I think the general approach seems OK - though my experience hacking around in the account manager is very limited. ::: mailnews/base/prefs/content/AccountManager.js @@ +1506,5 @@ > + > + } catch(e) { > + Components.utils.reportError("Error accessing account " + account + ": " + e); > + accountCorrect = false; > + accountName = "Invalid account " + accountName || accountKey; Needs to be localizable. @@ +1510,5 @@ > + accountName = "Invalid account " + accountName || accountKey; > + } > + > + // Create the top level tree-item > + var treeitem = document.createElement("treeitem"); let not var @@ +1523,1 @@ > if (panelsToKeep.length > 0) { Indentation here too, for everything in here. Or, this could be simplified I believe, as: if (accountCorrect) { treeitem.setAttribute("PageTag", AMChrome); } if (panelsToKeep.length > 0) { // ... }
Attachment #758278 -
Flags: feedback?(mconley) → feedback+
Assignee | ||
Comment 49•11 years ago
|
||
OK, what about this?
Attachment #758278 -
Attachment is obsolete: true
Attachment #758278 -
Flags: feedback?(iann_bugzilla)
Attachment #817288 -
Flags: review?(mkmelin+mozilla)
Attachment #817288 -
Flags: review?(iann_bugzilla)
Assignee | ||
Comment 50•11 years ago
|
||
Sorry, one more fix.
Attachment #817288 -
Attachment is obsolete: true
Attachment #817288 -
Flags: review?(mkmelin+mozilla)
Attachment #817288 -
Flags: review?(iann_bugzilla)
Attachment #817293 -
Flags: review?(mkmelin+mozilla)
Attachment #817293 -
Flags: review?(iann_bugzilla)
Comment 51•11 years ago
|
||
Comment on attachment 817293 [details] [diff] [review] patch v3 Review of attachment 817293 [details] [diff] [review]: ----------------------------------------------------------------- Looks reasonable to me. ::: mailnews/base/prefs/content/AccountManager.js @@ +1513,5 @@ > + amChrome = server ? server.accountManagerChrome : "am-main.xul"; > + } catch(e) { > + // Show only a placeholder in the account list saying this account > + // is broken, with no child panels. > + Components.utils.reportError("Error accessing account " + account + ": " + e); accountName?
Attachment #817293 -
Flags: review?(mkmelin+mozilla) → feedback+
Assignee | ||
Comment 52•11 years ago
|
||
OK. Magnus, do you intentionally do not want to review this? Can you propose another reviewer from TB?
Attachment #817293 -
Attachment is obsolete: true
Attachment #817293 -
Flags: review?(iann_bugzilla)
Attachment #821239 -
Flags: review?(mkmelin+mozilla)
Attachment #821239 -
Flags: review?(iann_bugzilla)
Comment 53•11 years ago
|
||
Comment on attachment 821239 [details] [diff] [review] patch v4 Review of attachment 821239 [details] [diff] [review]: ----------------------------------------------------------------- I'm not an official reviewer for /mailnews, and there's nothing /mail here.
Attachment #821239 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 54•11 years ago
|
||
Comment on attachment 821239 [details] [diff] [review] patch v4 Review of attachment 821239 [details] [diff] [review]: ----------------------------------------------------------------- OK.
Attachment #821239 -
Flags: review?(neil)
Comment 55•11 years ago
|
||
Please can you attach a -w version of the diff?
Comment 57•11 years ago
|
||
Comment on attachment 822367 [details] [diff] [review] patch v4 (diff -w) >+ if (!pageId) { >+ Components.utils.reportError("No pageId to show specified!"); Surely this is expected i.e. you have already reported the error? >+ let server = null; >+ let accountKey = null; >+ let amChrome = ""; >+ let panelsToKeep = []; >+ >+ // This "try {} catch {}" block is intentionally very long to catch >+ // unknown exceptions and confine them to this single account. >+ // This may happen from broken accounts. See e.g. bug 813929. >+ // Other accounts can still be shown properly if they are valid. >+ try { >+ server = account.incomingServer; You only use server inside the try block. >+ accountKey = account.key; But you should fetch the key first as you use it in the catch block. (Maybe fetch the key before the try, even.) >- var offline = server.offlineSupportLevel; >- var diskspace = server.supportsDiskSpace; >+ let offline = server.offlineSupportLevel; >+ let diskspace = server.supportsDiskSpace; [Silly var/let changes make the diff unnecessarily harder to read.] >+ amChrome = server ? server.accountManagerChrome : "am-main.xul"; Nit: server can't be null at this point, we have its pretty name.
Comment 58•11 years ago
|
||
Comment on attachment 821239 [details] [diff] [review] patch v4 >+++ b/mailnews/base/prefs/content/AccountManager.js >+ for (let account of accounts) { >+ let accountName = null; >+ let server = null; As said by Neil, this is only used within the try, so move it into there. >+ let accountKey = null; As said by Neil, you should probably set the accountKey here. >+ let amChrome = ""; Shouldn't this be set to "am-main.xul"? >+ // Now add our panels Nit: missing full stop. >+ // Check offline/diskspace support level Nit: missing full stop. >+ let offline = server.offlineSupportLevel; As this is only used once, it could be inlined. >+ let diskspace = server.supportsDiskSpace; >+ if (offline >= 10 && diskspace) >+ panelsToKeep.push(panels[2]); >+ else if (diskspace) >+ panelsToKeep.push(panels[3]); >+ >+ // extensions >+ let catMan = Components.classes["@mozilla.org/categorymanager;1"] >+ .getService(Ci.nsICategoryManager); Nit: line this up with the line above. >+ amChrome = server ? server.accountManagerChrome : "am-main.xul"; As Neil said, we know we have server here, so just set it to server.accountManagerChrome > // Create the top level tree-item Nit: missing full stop. > // Now add the outgoing server node Nit: missing full stop. r=me with those addressed / fixed.
Attachment #821239 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 59•11 years ago
|
||
(In reply to Ian Neal from comment #58) > >+ let amChrome = ""; > Shouldn't this be set to "am-main.xul"? > > >+ amChrome = server ? server.accountManagerChrome : "am-main.xul"; > As Neil said, we know we have server here, so just set it to > server.accountManagerChrome No, this would set the chrome to am-main.xul also for accounts that throw. I don't want to display the main pane for them. Who knows how broken they are if they can't even report their name. In that case I wanted to keep amChrome blank so that I can catch it in pageURL and show about:blank. But I can set it to about:blank here directly. Thanks for the other nits, I have fixed those.
Attachment #821239 -
Attachment is obsolete: true
Attachment #822367 -
Attachment is obsolete: true
Attachment #821239 -
Flags: review?(neil)
Attachment #822367 -
Flags: review?(neil)
Attachment #8338818 -
Flags: review?(iann_bugzilla)
Comment 60•10 years ago
|
||
Comment on attachment 8338818 [details] [diff] [review] patch v5 >+ try { >+ amChrome = server ? server.accountManagerChrome : "am-main.xul"; As previously said, we already know we have server at this point, so either this statement needs moving after the try/catch (maybe even inline) or you need to split it into the try/catch parts i.e. amChrome = server.accountManagerChrome and amChrome = "am-main.xul" >+ } catch(e) { >+ // Show only a placeholder in the account list saying this account >+ // is broken, with no child panels. >+ let accountID = (accountName || accountKey); >+ Components.utils.reportError("Error accessing account " + accountID + ": " + e); >+ accountName = "Invalid account " + accountID; >+ panelsToKeep.length = 0; >+ } >+ >+ // Create the top level tree-item. > var treeitem = document.createElement("treeitem"); > mainTree.appendChild(treeitem); > var treerow = document.createElement("treerow"); > treeitem.appendChild(treerow); > var treecell = document.createElement("treecell"); > treerow.appendChild(treecell); >+ treecell.setAttribute("label", accountName); >+ treeitem.setAttribute("PageTag", amChrome); r=me with that fixed
Attachment #8338818 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 61•10 years ago
|
||
OK, I see what you are up to. The server must not throw and must not be null at that point we assign amChrome. But then that check was useless even before my patch as a null server would throw much sooner above, at the server.type and other references of its properties.
Attachment #8338818 -
Attachment is obsolete: true
Attachment #8345481 -
Flags: review+
Assignee | ||
Comment 62•10 years ago
|
||
Comment on attachment 8345481 [details] [diff] [review] patch v6 Review of attachment 8345481 [details] [diff] [review]: ----------------------------------------------------------------- OK, thanks. So let's get a full mailnews r+.
Attachment #8345481 -
Flags: review+ → review?(neil)
Comment 63•10 years ago
|
||
Comment on attachment 8345481 [details] [diff] [review] patch v6 > function pageURL(pageId) > { >+ // If we have a special non account manager pane (e.g. about:blank), >+ // do not translate it into ChromePackageName URL. >+ if (pageId.substr(0,3) != "am-") >+ return pageId; Use .startsWith instead. r=me with that fixed. [I didn't have a problem with the null pageId special case before, I just didn't see why you needed to report an error when you were expecting it. Also you forgot the space after comma.]
Attachment #8345481 -
Flags: review?(neil) → review+
Assignee | ||
Comment 64•10 years ago
|
||
Thanks, fixed the nits.
Attachment #8345481 -
Attachment is obsolete: true
Attachment #8362663 -
Flags: review+
Comment 65•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/b4cb7f82793b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in
before you can comment on or make changes to this bug.
Description
•