259193 - Account Manager / Wizard should prompt for more valid fields

Status: RESOLVED WORKSFORME

(MailNews Core :: Account Manager, enhancement)

Description

[Ian Neal]

Currently Account Manager only prompts you to enter a valid smtp server hostname
and ignores invalid other hostnames, usernames and email addresses.
Similarly Account Wizard does not check for a valid username (if required) other
than a blank one.

Comment 1

[Ian Neal]

Attachment: Patch v0.1 first pass at a fix

Renamed gSmtpHostNameIsIllegal to gSomethingIllegal
Tweaks selectServer function so that it can be used to fix bug 259187 - removed
updateButtons call as the select calls it anyway.
Added checks for valid email addresses, usernames and non-smtp hostnames.
Only call updateButtons if changing to another account - showPage was tweaked
to return true if switching accounts.
Moved containsIllegalChar from aw-identity.js to amUtils.js
Added additional check to username in aw-login.js
Tweaked error message generated by an invalid username to match those for email
address and hostname.

Comment 2

[Ian Neal]

Comment on attachment 158820 [details] [diff] [review]
Patch v0.1 first pass at a fix

Cancelling request as splitting the fix to bug 259187 out and getting that in
first

Comment 3

[Ian Neal]

Attachment: Revised patch (v0.2) following checkin of bug 259187

Renamed gSmtpHostNameIsIllegal to gSomethingIsIllegal
Added checks for valid email addresses, usernames and non-smtp hostnames.
Moved containsIllegalChar from aw-identity.js to amUtils.js
Added additional check to username in aw-login.js
Tweaked error message generated by an invalid username to match those for email

address and hostname.

There may be some scope for combining the hostname/username checks

Comment 4

[Serge Gautherie (:sgautherie)]

Ian, Neil,
Are you still working on this ?

Comment 5

[Ian Neal]

Was waiting for a review...

Comment 6

[Thomas D. (:thomas8)]

Ian, looking at Neil's review queue, I think it's crucial to specify if this patch is still applicable or not. If it's no longer applicable, pls mark patch obsolete and cancel review request, so that we can have some focus on current reviews. Tia.

Comment 7

[Thomas D. (:thomas8)]

(In reply to Thomas D. (away till 31st January) from comment #6)
> Ian, looking at Neil's review queue [1], I think it's crucial to specify if this
> patch is still applicable or not. If it's no longer applicable, pls mark
> patch obsolete and cancel review request, so that we can have some focus on
> current reviews. Tia.

[1] https://bugzilla.mozilla.org/request.cgi?action=queue&type=review&requestee=neil%40httl.net&group=type

Comment 8

[Ian Neal]

Comment on attachment 161719 [details] [diff] [review]
Revised patch (v0.2) following checkin of bug 259187

This patch is completely bitrotted, some of the work has now been done in various other bugs. Let's see if aceman thinks there is anything still required to be done.

Comment 9

[:aceman]

In the last patch I do not see any changes to aw-* files as mentioned in comment 3. Is patch v0.2 incomplete?
But according to the changes to AccountManager.js I can see this in current trunk:
- hostname is already checked, better than in patch v0.1 (up to domain validity)
- username is checked only against being empty. If you want to reject the invalid chars, you could still add that check.
- email is not checked and can even be left empty. Not sure what happens if we send out and email with such an identity. If any check is added, it should probably be done on each identity (the main pane and also am-identity-edit.js)

Comment 10

[Wayne Mery (:wsmwk)]

bug still useful?

Comment 11

[Magnus Melin [:mkmelin]]

Nope.