Last Comment Bug 218439 - Account manager should disallow setting local directory of multiple accounts to the same physical directory (two accounts with same local directory causes mozilla to crash when checking IMAP mail or regenerate msf files)
: Account manager should disallow setting local directory of multiple accounts ...
Status: VERIFIED FIXED
: crash, stackwanted
Product: MailNews Core
Classification: Components
Component: Account Manager (show other bugs)
: Trunk
: All All
: -- critical (vote)
: Thunderbird 13.0
Assigned To: :aceman
:
:
Mentors:
: 59794 losingmails 237880 720285 (view as bug list)
Depends on:
Blocks: losingmails 379663 577775 720285
  Show dependency treegraph
 
Reported: 2003-09-05 10:42 PDT by Mirek Hankus
Modified: 2012-05-21 11:35 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
fix stringbundle (5.08 KB, patch)
2012-01-26 12:04 PST, :aceman
iann_bugzilla: review-
Details | Diff | Splinter Review
use bundles from top documents (2.89 KB, patch)
2012-01-27 12:14 PST, :aceman
iann_bugzilla: review+
standard8: review+
Details | Diff | Splinter Review

Description Mirek Hankus 2003-09-05 10:42:54 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030904
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030904

When checking mail list of all mail headers disappears and after about 1 second 
mozilla crashes with folowing backtrace

#0  0x417abadc in NSGetModule () from /usr/local/mozilla/components/libmsgdb.so
#1  0x417acc81 in NSGetModule () from /usr/local/mozilla/components/libmsgdb.so
#2  0x416e7a64 in NSGetModule () from /usr/local/mozilla/components/libmsgimap.so
#3  0x405d2a1f in XPTC_InvokeByIndex () from /usr/local/mozilla/libxpcom.so
#4  0x405bda8f in nsProxyObject::Post(unsigned, nsXPTMethodInfo*,
nsXPTCMiniVariant*, nsIInterfaceInfo*) () from /usr/local/mozilla/libxpcom.so
#5  0x405b5647 in PL_HandleEvent () from /usr/local/mozilla/libxpcom.so
#6  0x405b5574 in PL_ProcessPendingEvents () from /usr/local/mozilla/libxpcom.so
#7  0x405b690b in nsEventQueueImpl::ProcessPendingEvents() () from
/usr/local/mozilla/libxpcom.so
#8  0x40da8135 in parent_class_set_focus () from
/usr/local/mozilla/components/libwidget_gtk.so
#9  0x40da7ced in parent_class_set_focus () from
/usr/local/mozilla/components/libwidget_gtk.so
#10 0x4026a049 in g_io_unix_dispatch () from /usr/lib/libglib-1.2.so.0
#11 0x4026b806 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#12 0x4026be33 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#13 0x4026bfec in g_main_run () from /usr/lib/libglib-1.2.so.0
#14 0x4018c6ab in gtk_main () from /usr/X11R6/lib/libgtk-1.2.so.0
#15 0x40da8536 in parent_class_set_focus () from
/usr/local/mozilla/components/libwidget_gtk.so
#16 0x40d853c4 in NSGetModule () from /usr/local/mozilla/components/libnsappshell.so
#17 0x0805b044 in getCountry(nsAString const&, nsAString&) ()
#18 0x0805b886 in main ()
#19 0x4038b3dd in __libc_start_main () from /lib/libc.so.6


I've got two different IMAP accounts on the same server (UW IMAP), so problem
may be related to having two accounts on one server

Reproducible: Always

Steps to Reproduce:
1.check mail 
2.
3.

Actual Results:  
mozilla crashed

Expected Results:  
mozilla should not crash
Comment 1 David :Bienvenu 2003-09-11 08:20:46 PDT
can you attach or e-mail me your prefs.js so I can see your account setup? thx.
Comment 2 David :Bienvenu 2003-09-11 16:00:12 PDT
I suspect this is because Mirek has two imap server accounts with the same local
mail directory. That doesn't and can't work well, and should be prevented in the
UI and/or backend.
Comment 3 Mirek Hankus 2003-09-12 13:31:37 PDT
Changing local directory for one of the accounts fixed the problem, 

Now checking mail does not crash mozilla. 
Anyway mozilla should detect situation when two accounts are located 
on the same server and store info abount them in different directories
Comment 4 David :Bienvenu 2003-09-12 13:36:20 PDT
yes, it should - and it should prevent the user from specifying the same local
directory for two different accounts, and create two different local directories
if there are two accounts with the same server name.
Comment 5 David :Bienvenu 2003-10-01 08:37:15 PDT
changing summary
Comment 6 WADA 2005-12-19 01:37:18 PST
When I try to choose directory of "Local Folders" for a POP3 account,
and when I try to choose directory of a POP3 account for "Local Folders" account,
Thunderbird version 1.6a1 (20051206) kindly issued following message.
> This directory is already used by another server.
> Please pick a different directry.
Then choosing already used directry was impossible.
I think this warning is issued by UI logic, and I guess this bug(crash) will still remain if user set duplicate directory in prefs.js by text editor or config-editor, but I think this kind of change(=manual corruption of prefs.js) is "User Error", then I belive this bug can be closed as WORKSFORME(or FIXED).
Comment 7 Wayne Mery (:wsmwk, NI for questions) 2010-01-25 04:48:08 PST
bienvenu, ludo, standard8, thoughts on comment 6?
note bug 379663.
Comment 8 Wayne Mery (:wsmwk, NI for questions) 2010-05-15 06:39:16 PDT
would be nice to have a breakpad stack sig to add to the summary
Comment 9 :aceman 2012-01-26 02:31:19 PST
I can try to warn about this in the account manager. That would be a fix according to comment 6. I think if would also detect user editing of prefs.js but only when the user goes into the account manager for the first time after that change. That could be good enough.

WADA could you please help me where to find code of Thunderbird version 1.6a1 (20051206) so I can look how it detected this state?

David Bienvenu, are you working on this, or have you been just assigned automatically?
Comment 10 :aceman 2012-01-26 02:35:12 PST
For the record, this seems to be the recommended solution for somebody wanting messages from several accounts to land in one Inbox, without reusing the same Local Directory:

You can do this simply by deferring the storage for the second account to the 1st account (or defer all of them to the global inbox). See Account settings, Server settings, Advanced button for how to defer the storage of an account to an other account's inbox.
Comment 11 WADA 2012-01-26 02:41:45 PST
(In reply to :aceman from comment #9)
> where to find code of Thunderbird version 1.6a1 (20051206) so I can look how it detected this state?

Binary? Source? If binary for testing, see next directory.
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/2005/12/
Comment 12 :aceman 2012-01-26 02:50:54 PST
Source please so I can maybe reuse it.
Comment 13 :aceman 2012-01-26 02:55:29 PST
*** Bug 59794 has been marked as a duplicate of this bug. ***
Comment 15 :aceman 2012-01-26 03:23:35 PST
Thanks Magnus. It seems the code is still there today, but does not alert the user, just silently does not change the directory. I'll look at it.
Comment 16 :aceman 2012-01-26 03:26:30 PST
Well, no wonder :)

Timestamp: 26.1.2012 12:21:44
Error: top.gPrefsBundle is undefined
Source File: chrome://messenger/content/amUtils.js
Line: 75
Comment 17 :aceman 2012-01-26 06:07:51 PST
*** Bug 237880 has been marked as a duplicate of this bug. ***
Comment 18 :aceman 2012-01-26 11:50:58 PST
Any idea why the title string for the filepicker in amUtils.js is passed in this way (in filepickertitle attribute)?
<button id="browseForLocalFolder" label="&browseFolder.label;" filepickertitle="&localFolderPicker.label;" ... >

Can I move them to normal prefs.properties?
Comment 19 :aceman 2012-01-26 12:04:24 PST
Created attachment 591885 [details] [diff] [review]
fix stringbundle

This is a quick fix.

However I think there could be some reorganization of the strings done. Some strings could be moved from messenger.properties to prefs.properties and then maybe messenger.properties could be dropped from some of the files. Would that be useful as another bug? Could it save some memory?
Comment 20 Ian Neal 2012-01-26 17:27:22 PST
Comment on attachment 591885 [details] [diff] [review]
fix stringbundle

>+++ b/mailnews/base/prefs/content/am-server.xul
>+    <stringbundle id="bundle_prefs" src="chrome://messenger/locale/prefs.properties"/>

>+++ b/mailnews/base/prefs/content/am-serverwithnoidentities.xul
>+  <stringbundle id="bundle_prefs" src="chrome://messenger/locale/prefs.properties"/>

>+++ b/mailnews/base/prefs/content/amUtils.js
>-        var directoryAlreadyUsed =
>-          top.gPrefsBundle.getFormattedString(
>-            "directoryUsedByOtherAccount", [ currentServer.prettyName ]);
>+        var directoryAlreadyUsed = document.getElementById("bundle_prefs")
>+                                   .getFormattedString("directoryUsedByOtherAccount",
>+                                                       [currentServer.prettyName]);
Instead of adding the <stringbundle>s to the two xul files you could just use top.document.getElementById("bundle_prefs")

In fact, you could probably do that across all the am-*.xul and aw-*.xul files as all the top level xul files (AccoutManager.xul, AccountWizard.xul and msgAccountCentral.xul) already have the relevant <stringbundle> though it is worth testing first before creating a spin-off bug.
Comment 21 :aceman 2012-01-27 00:18:25 PST
Thanks I can try that. Can bundle_messenger be used in the same way?
Comment 22 :aceman 2012-01-27 12:14:28 PST
Created attachment 592209 [details] [diff] [review]
use bundles from top documents

It seems you are right. I tried to change also smtpserveredit and smtpeditoverlay. It seems the bundles must be included in each <!DOCTYPE dialog>
but not in <!DOCTYPE page>. The page can reference the bundle via top.document.getElementId("bundle_name").

Would there be any gain when the bundles were removed from the <!DOCTYPE page> files?
Comment 23 Ian Neal 2012-01-27 16:39:57 PST
(In reply to :aceman from comment #21)
> Thanks I can try that. Can bundle_messenger be used in the same way?
Perhaps, in places, but it was probably less straight forward.

(In reply to :aceman from comment #22)
> Created attachment 592209 [details] [diff] [review]
> use bundles from top documents
> 
> It seems you are right. I tried to change also smtpserveredit and
> smtpeditoverlay. It seems the bundles must be included in each <!DOCTYPE
> dialog>
> but not in <!DOCTYPE page>. The page can reference the bundle via
> top.document.getElementId("bundle_name").
Pages are within a wizard, and the top is the wizard which has the bundle in.

> Would there be any gain when the bundles were removed from the <!DOCTYPE
> page> files?
Good question, I don't know the answer, perhaps Mark, Neil or someone else on the cc list does?
Comment 24 Mark Banner (:standard8) 2012-02-21 03:26:19 PST
(In reply to Ian Neal from comment #23)
> (In reply to :aceman from comment #21)
> > Would there be any gain when the bundles were removed from the <!DOCTYPE
> > page> files?
> Good question, I don't know the answer, perhaps Mark, Neil or someone else
> on the cc list does?

I'm not sure myself. I suspect there would be some minimal improvement at least, even if its just in code readability.
Comment 25 :aceman 2012-02-21 03:43:52 PST
I have made some tests (like putting all the 170KB of .properties files into prefs.js) but couldn't see any memory increase. So either it is insignificant or the strings are not loaded at once (but only as needed) or I couldn't read output of top properly.

I had in mind something like putting brand, messenger, prefs bundles only in the top accountManager.xul and let all the other subpanes access them via top.document. I don't know if it would be slower (top.document.getelement vs. document.getelement) but could save some memory.

I do not know now if it is worth pursuing.
Comment 26 Mark Banner (:standard8) 2012-02-21 04:35:11 PST
Checked in: http://hg.mozilla.org/comm-central/rev/61c6cbd61566
Comment 27 Ian Neal 2012-02-21 07:59:54 PST
(In reply to :aceman from comment #25)
> I had in mind something like putting brand, messenger, prefs bundles only in
> the top accountManager.xul and let all the other subpanes access them via
> top.document. I don't know if it would be slower (top.document.getelement
> vs. document.getelement) but could save some memory.
> 
> I do not know now if it is worth pursuing.

AccountManager.xul already has stringbundles for brand and prefs, so just a matter of adding messenger. Might be worth putting them all into a stringbundleset to make the life of add-on authors easier when it comes to overlaying. This of course is all food for a spin-off bug.
Comment 28 :aceman 2012-02-21 08:09:59 PST
Of course a new bug. I just wonder if it is worth to file it and do something like the proposed.
I do not know what stringbudleset is good for. So if any of you want to file the bug and propose some cleanup there that would surely bring some positive gain then I could play with it.
Comment 29 :aceman 2012-04-10 03:29:03 PDT
*** Bug 720285 has been marked as a duplicate of this bug. ***
Comment 30 David :Bienvenu 2012-05-04 13:12:57 PDT
*** Bug 147711 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.