107833 - Full prefs UI support for movemail accounts

Status: RESOLVED FIXED

(MailNews Core :: Movemail, defect)

Description

[Peter O'Shea]

Mozilla should have full support for movemail accounts in the preferences UI. 
This is a basic functionality that was in Communicator and it's hard to believe
it's still not there, while there's lots of pretty tabbed browsing and sidebar
stuff in the browser.

IMHO, this should be a requirement for 1.0.  You shouldn't have to hand-edit
prefs.js to get movemail working.

Comment 1

[Boris Zbarsky [:bzbarsky]]

duplicate of "

Comment 2

[Peter O'Shea]

Adding the mozilla1.0 keyword.  It seems to me that basic functionality that was
in 4.7x is a no-brainer for moz1.0.  

Comment 3

[Adam D. Moss]

I can't commit to doing this myself; reassigning to nobody
and futuring + helpwanted.

Comment 4

[Adam D. Moss]

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

Comment 5

[Adam D. Moss]

Bug 85360 has been marked as a duplicate of this one
by virtue of this bug's goal (and summary) being a lot clearer:
provide a UI by which movemail accounts may be created
and administered.

Comment 6

[[e-mail]]

Taking for now. I guess I can come up with something that will essentially
provide the frontent for setting preferences as for a pop3 account but
with a fixed incoming server "localhost" and type "movemail", which should
work.

Helpwanted about what preferences really are possible and which values they
can have for the movemail backend.

Also, for which OS should this be supported?

Removing obsolete mozilla1.0 keyword.

Comment 7

[[e-mail]]

Oops, wrong assign.

BTW how is this related to bug 56669?

Comment 8

[Adam D. Moss]

The original movemail patch I did (probably still attached
to the original movemail bug, wherever that is) did have
more-or-less adequate UI support for the account wizard and
prefs.  BUT it turned out that there wasn't an elegant way
to just 'plug in' support for a new account type that only
exists on one platform (unix) without lots of #ifdefs and
the JS equivilent, so that part of movemail support got
dropped and this is where we are.

That's primarily how this relates to bug 56669.

Comment 9

[Roland Mainz]

petez:
Wanna take a look at this one, please ?

"movemail" support is Mozilla's way to fetch the emails from /var/mail/$LOGNAME
directly (without setting-up a POP3/IMAP daemon first) and would be required to
make Mozilla a full replacement for /usr/dt/bin/dtmail ...

Comment 10

[[e-mail]]

Adam I think any patch that would give the user a way to do this
would be *much* better than the current situation of no-support-at all?
I dont think that the account manager extension is really a requirement for
this - if somebody comes up with it and its use for movemail, all the 
better, we can then back out the simple patch without problems.

I do not think that the issue of not having movemail on non-UNIX OS is a big
issue: we could simply hide that type in the creation wizard for these OS'.
So there will never be an option for creation visible and since no such
account can be created, the edit dialog will not be visible ever either.

So do you think you could dig up that old patch somewhere?

Comment 11

[Adam D. Moss]

The original patch with the rough UI support is attached
to the original movemail bug:

http://bugzilla.mozilla.org/show_bug.cgi?id=11049

... though there's not much chance of it applying to a
modern tree, and I long ago stopped updating the changes
against the trunk.

Still, reading the patch and looking at the new files it
should be fairly obvious what needs to be done for movemail
UI support (minus the unix-only detection).  IIRC I trawled
through the wizard and account manager stuff simply looking
for pop3 references and then adding an accompying movemail
reference.

Comment 12

[[e-mail]]

It seems that preference "mail.use_movemail" gets predefined only
on those platforms where movemail is available. I guess that should suffice
for the hiding test.

Comment 13

[(not reading, please use [e-mail] instead)]

"mail.use_movemail" looks like unused cruft from 4.x

right now, movemail support is something that happens at build time.

Comment 14

[[e-mail]]

ok, but obviously that preference seems to reflect quite well on which platform
movemail support is available (unix, but not openvms). If this should get
dropped, ist there another way to find out if movemail is "built in" from
XUL/JS? If not, why not keep this method?

Comment 15

[Philip K. Warren]

I am working with Seth on trying to implement this functionality through the ISP
default files (like aol.rdf and nswebmail.rdf that are shipped with Netscape 7).
To view a sample of these files, look at the following:

http://lxr.mozilla.org/seamonkey/source/mailnews/base/ispdata/aol.rdf

Right now these files don't work exactly as I'd like for movemail accounts, in
particular:

1) I want to create a default SMTP server 'localhost' only if no SMTP server is
specified. There is the setting 'smtpCreateNewServer' in the RDF files, but it
doesn't seem to work properly. I believe the following lines of code are the
reason why:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/base/prefs/resources/content/AccountWizard.js&rev=&root=/cvsroot&mark=451-460

Instead of using the smtpCreateNewServer preference as the comment describes, it
only checks if the smtp server name in non-null.

2) The user should not be able to create two movemail accounts in the same
profile. I don't believe this functionality currently exists.

3) Nit: The current RDF ispdata functionality does not properly create a
movemail account if the <NC:hostName> value is not set. There should be an
exception for movemail accounts so they do not need to specify a hostname.

I will post a movemail.rdf which is what I currently am working with. To test
the configuration, create the directory defaults/isp and defaults/isp/US in your
Mozilla directory and copy the rdf file into each directory.

Comment 16

[Philip K. Warren]

Attachment: movemail.rdf v1

Comment 17

[Daniel Küstner]

Someone is working on it. Great!

Some UI issues:

When I create a new account there is an arbitrary string added as domain name.

With POP3 and IMAP accounts I have the ability to set the account name at the
end of the creation process. It would be nice to have the possibility also for
movemail.

The name UNIX movemail as an _account name_ seems to me not perfectly, since I
know that I am working on a unix system and a windows user won't get it.

UNIX movemail as a _category_ between email and newsgroup account is
problematic, since it is an email account. IMHO it'd be better to have it as
category beneath POP3 and IMAP. Then one needed to have the type of email
account asked before the email address, but it would be cleaner.

Comment 18

[Roland Mainz]

Daniel Küstner wrote:
> The name UNIX movemail as an _account name_ seems to me not perfectly, since I
> know that I am working on a unix system and a windows user won't get it.

Well, what about using the file name where we fetch the files from as account
name (e.g. "/var/mail/rmainz", "/var/mail/gisburn", etc.) ?

Comment 19

[Philip K. Warren]

> When I create a new account there is an arbitrary string added as domain name.

This is fixed with the checkin for Bug 193376.

> With POP3 and IMAP accounts I have the ability to set the account name at the
> end of the creation process. It would be nice to have the possibility also for
> movemail.

I will look into this.

> The name UNIX movemail as an _account name_ seems to me not perfectly, since I
> know that I am working on a unix system and a windows user won't get it.

I took the current name from here:
http://lxr.mozilla.org/seamonkey/source/mailnews/base/resources/locale/en-US/messenger.properties#106.

I would definitely be willing to change it to something else if needed.

> UNIX movemail as a _category_ between email and newsgroup account is
> problematic, since it is an email account. IMHO it'd be better to have it as
> category beneath POP3 and IMAP. Then one needed to have the type of email
> account asked before the email address, but it would be cleaner.

I'm not sure if it is possible to control the order in which items appear in the
.rdf ISP files. Seth - would this be possible?

Comment 20

[Roland Mainz]

pkw:
Should I reassign this bug to you ?

Comment 21

[Philip K. Warren]

Taking ownership of bug.

Comment 22

[Philip K. Warren]

Attachment: Patch v1

Patch to enable movemail prefs support through the ISP .rdf interface. Read the
following annotation of the patch for more information on the implementation.

Comment 23

[Philip K. Warren]

Attachment: Patch v1 annotation

Comment 24

[Philip K. Warren]

I have found an error in this version of the patch. At the end of the account
creation, it is not setting the following preference for Movemail accounts, so
on subsequent startups, the Movemail account does not work correctly:

user_pref("mail.server.server1.hostname", "localhost");

I will post an updated patch soon which fixes this problem.

Comment 25

[Philip K. Warren]

Attachment: Patch v2

Fixes issue described in comment 24.

Comment 26

[(not reading, please use [e-mail] instead)]

philip, your patch annotation made reviewing a snap.  thanks for spending the
time and doing that.

two style suggestions:

1)

+    // Find out if we need to hide details for incoming servers
+    var hideIncoming = false;
+    if (gCurrentAccountData && gCurrentAccountData.wizardHideIncoming)
+        hideIncoming = true;

why not just:

// Find out if we need to hide details for incoming servers
var hideIncoming = (gCurrentAccountData && gCurrentAccountData.wizardHideIncoming);

2)

+    // Hide the "username" field if we don't want to show information
+    // on the incoming server.
+    if (hideIncoming)
+        setDivTextFromForm("server.username", null);
+    else
+        setDivTextFromForm("server.username", userName);

why not just:

// Hide the "username" field if we don't want to show information
// on the incoming server.
setDivTextFromForm("server.username", hideIncoming ? null : userName)

Comment 27

[(not reading, please use [e-mail] instead)]

Comment on attachment 126062 [details] [diff] [review]
Patch v2

r/sr=sspitzer, but first:

1)  please make those style changes I mentioned in the last comment.

2)  do you think we should export movemail.rdf for linux as well as AIX?

3)  you've made some changes to movemail.rdf, I bet there's some cruft in
aol.rdf and nswebmail.rdf (that are in the moz tree, for example purposes). 
can you eyeball them, and update them to be "valid"?

4)  please make sure to test that news, pop, imap all work fine.

Comment 28

[Philip K. Warren]

1)  please make those style changes I mentioned in the last comment.

Will do. I will post a new patch with only these changes.

2)  do you think we should export movemail.rdf for linux as well as AIX?

I didn't want to step on anyone's toes with this patch, but I am happy to enable
building movemail.rdf for other platforms. If we do make this change for Linux
as well, we should probably update packages-unix to ship the movemail.rdf file
(if it was built) for installer builds as well.

3)  you've made some changes to movemail.rdf, I bet there's some cruft in
aol.rdf and nswebmail.rdf (that are in the moz tree, for example purposes). 
can you eyeball them, and update them to be "valid"?

I made sure that any of the changes I made to the RDF syntax would be backwards
compatible with the older versions of the rdf files as well. I don't see any
problems with these currently, but it would probably make sense to document the
syntax of the ISP RDF files somewhere.

4)  please make sure to test that news, pop, imap all work fine.

Comment 29

[Philip K. Warren]

Fixed.

Checking in Makefile.in;
/cvsroot/mozilla/mailnews/base/Makefile.in,v  <--  Makefile.in
new revision: 1.15; previous revision: 1.14
done
Checking in ispdata/Makefile.in;
/cvsroot/mozilla/mailnews/base/ispdata/Makefile.in,v  <--  Makefile.in
new revision: 1.6; previous revision: 1.5
done
Checking in ispdata/movemail.rdf;
/cvsroot/mozilla/mailnews/base/ispdata/movemail.rdf,v  <--  movemail.rdf
new revision: 1.2; previous revision: 1.1
done
Checking in prefs/resources/content/AccountWizard.js;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/AccountWizard.js,v  <-- 
AccountWizard.js
new revision: 1.115; previous revision: 1.114
done
Checking in prefs/resources/content/AccountWizard.xul;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/AccountWizard.xul,v  <--
 AccountWizard.xul
new revision: 1.52; previous revision: 1.51
done
Checking in prefs/resources/content/aw-done.js;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/aw-done.js,v  <--  aw-done.js
new revision: 1.18; previous revision: 1.17
done
Checking in prefs/resources/content/aw-identity.js;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/aw-identity.js,v  <-- 
aw-identity.js
new revision: 1.23; previous revision: 1.22
done
Checking in prefs/resources/content/aw-server.js;
/cvsroot/mozilla/mailnews/base/prefs/resources/content/aw-server.js,v  <-- 
aw-server.js
new revision: 1.37; previous revision: 1.36
done

Comment 30

[Philip K. Warren]

This patch currently enables creating Movemail accounts from the Account Wizard
for AIX systems by default. I have opened Bug 212804 in order to extend this to
other UNIX platforms.