Closed Bug 218439 Opened 17 years ago Closed 9 years ago

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)

Categories

(MailNews Core :: Account Manager, defect)

defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird 13.0

People

(Reporter: M.Hankus, Assigned: aceman)

References

Details

(Keywords: crash, stackwanted)

Attachments

(1 file, 1 obsolete file)

2.89 KB, patch
iann_bugzilla
: review+
standard8
: review+
Details | Diff | Splinter Review
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
Keywords: crash
can you attach or e-mail me your prefs.js so I can see your account setup? thx.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
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
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.
changing summary
Summary: mozilla crashes while checking IMAP mail → two accounts with same local directory causes mozilla to crash when checking IMAP mail
Product: MailNews → Core
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).
Product: Core → MailNews Core
QA Contact: grylchan → networking.imap
bienvenu, ludo, standard8, thoughts on comment 6?
note bug 379663.
would be nice to have a breakpad stack sig to add to the summary
Keywords: stackwanted
Blocks: 720285
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?
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.
(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/
Source please so I can maybe reuse it.
Duplicate of this bug: 59794
Component: Networking: IMAP → Account Manager
OS: Linux → All
QA Contact: networking.imap → account-manager
Hardware: x86 → All
Summary: two accounts with same local directory causes mozilla to crash when checking IMAP mail → 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)
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.
Well, no wonder :)

Timestamp: 26.1.2012 12:21:44
Error: top.gPrefsBundle is undefined
Source File: chrome://messenger/content/amUtils.js
Line: 75
Assignee: dbienvenu → acelists
Duplicate of this bug: 237880
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?
Attached patch fix stringbundle (obsolete) — Splinter Review
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?
Attachment #591885 - Flags: ui-review?(bwinton)
Attachment #591885 - Flags: review?(iann_bugzilla)
Status: NEW → ASSIGNED
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.
Attachment #591885 - Flags: review?(iann_bugzilla) → review-
Thanks I can try that. Can bundle_messenger be used in the same way?
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?
Attachment #591885 - Attachment is obsolete: true
Attachment #592209 - Flags: ui-review?(mbanner)
Attachment #592209 - Flags: review?(iann_bugzilla)
Attachment #591885 - Flags: ui-review?(bwinton)
Attachment #592209 - Flags: ui-review?(mbanner) → review?(mbanner)
Blocks: losingmails
(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?
Attachment #592209 - Flags: review?(iann_bugzilla) → review+
Blocks: 577775
Attachment #592209 - Flags: review?(mbanner) → review+
(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.
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.
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/61c6cbd61566
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
(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.
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.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 720285
Duplicate of this bug: losingmails
No longer blocks: 577775
Blocks: 577775
You need to log in before you can comment on or make changes to this bug.