314995 - Can't save new card if no email entered

Status: VERIFIED FIXED

(MailNews Core :: Address Book, defect)

Description

[Eric Rosenbloom]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051005 SeaMonkey/1.1a
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051005 SeaMonkey/1.1a

If I create a new card thatt is missing an email address, SeaMonkey won't let me save it. I have to enter something like [e-mail] to be able to save it. Pre-SeaMonkey Mozilla allowed saving email-less contacts.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

We implemented this intentionally.

Comment 2

[Eric Rosenbloom]

I'm sure there was a very good reason, but it diminishes the usefulness of the address book as a general contact list.

Comment 3

[Wayne Mery (:wsmwk)]

I saw a bug to address this, recently fixed I thought. can't find it - is it under thunderbird?

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

See bug 64305.  I won't mark it a DUP, since in all fairness this could be turned into an RFE (request for enhancement), but unless the module owner agreed to change this, it'll probably remain this way.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

I would argue that we should, in fact, allow cards without email addresses for the reason that the original poster suggested.  Perhaps there should be a set of required fields that any given card must have at least one of?

Comment 6

[Mark Banner (:standard8)]

I've been discussing this with Dan on irc and I agree that we limited the required fields too much by always requiring an email address.

The revised proposal is to require at least one of the following fields to be entered on the card:

email address, first name, last name, display name, organization (company name).

This is hopefully a more sensible list that will still ensure we don't allow blank cards, but will allow flexibility as to what data is actually displayed.

We should however also make sure autocomplete doesn't pick up cards with no email address (a separate bug if it does).

The other issue which is partially why it was requiring an email address on every card was the way in which we handle cards on mailing lists - the cards are not correctly added if they don't have an email address. We need work elsewhere to not allow cards in mailing lists if they don't have an email address - namely as it doesn't make sense. I'll hive the mailing lists issue off into a seperate bug, but wanted to mention it here for completeness.

Comment 7

[Mark Banner (:standard8)]

Attachment: Allow more than just email address as a minimum entry requirement

This patch expands the requirement for just one field (primary email address) as a minimum entry to an address book card to multiple fields (see comment #6).

There will need to be a follow-up for the card view so that we display something correctly in all cases, but that can be handled under bug 126491.

Comment 8

[Scott MacGregor]

From the peanut gallery, is this really something important to offer and support? I'm having a hard time seeing lots of people storing addressbook cards in an e-mail client for users that don't have e-mail addresses. 

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

mscott: some people want to keep all their addressbook data (email and otherwise) in one place.  This seems like a good use case to care about.

Comment 10

[Theblueman]

Hoping this can be resolved quickly.  I use mozilla address book as my contact manager and also as a data source for OpenOffice 1.1.4 to do mail merges.  Definitely limits my use.

Comment 11

[[e-mail]]

Comment on attachment 203841 [details] [diff] [review]
Allow more than just email address as a minimum entry requirement

>+      doc.getElementById("FirstName").value.length == 0 &&
>+      doc.getElementById("LastName").value.length == 0 &&
>+      doc.getElementById("DisplayName").value.length == 0 &&
>+      doc.getElementById("Company").value.length == 0)
Please change these to use .textLength

>+  var primaryEmailValueAtLastIndex = primaryEmailValue.lastIndexOf("@");
>+  if (!((primaryEmailValueLength == 0) ||
>+        ((primaryEmailValueLength >= 3) &&
>+         (primaryEmailValueAtLastIndex > 0) &&
>+         (primaryEmailValueAtLastIndex < primaryEmailValueLength - 1))))
It seems to me that there should be an easier way to write this, maybe using a regexp.

Comment 12

[Mark Banner (:standard8)]

Patch checked in with Neil's comments addressed.

Checking in mailnews/addrbook/resources/content/abCardOverlay.js;
/cvsroot/mozilla/mailnews/addrbook/resources/content/abCardOverlay.js,v  <--  abCardOverlay.js
new revision: 1.59; previous revision: 1.58
done
Checking in mailnews/addrbook/resources/locale/en-US/addressBook.properties;
/cvsroot/mozilla/mailnews/addrbook/resources/locale/en-US/addressBook.properties,v  <--  addressBook.properties
new revision: 1.36; previous revision: 1.35
done
Checking in mail/components/addrbook/content/abCardOverlay.js;
/cvsroot/mozilla/mail/components/addrbook/content/abCardOverlay.js,v  <--  abCardOverlay.js
new revision: 1.11; previous revision: 1.10
done
Checking in mail/locales/en-US/chrome/messenger/addressbook/addressBook.properties;
/cvsroot/mozilla/mail/locales/en-US/chrome/messenger/addressbook/addressBook.properties,v  <--  addressBook.properties
new revision: 1.8; previous revision: 1.7

Comment 13

[Serge Gautherie (:sgautherie)]

(In reply to comment #12)
> Patch checked in with Neil's comments addressed.

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051127 SeaMonkey/1.5a] (nightly) (W98SE)

|if (primaryEmail.textLength != 0 && !/.@./.test(primaryEmail.value))|

Nit:
In case we would care, this regexp is a regression compared to the previous tests:
an address like "@@@" is now accepted, whereas we previously required at least "@@h".
May be it would not be too hard to actually check for an "u@h" format, which would eventually be consistent with the help text.

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

version 1.6a1 (20051202) of Thunderbird trunk on Windows XP

and

SeaMonkey 1.5a trunk;Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051202 Mozilla/1.0

I double-checked all of the relevant fields.

Comment 15

[Ian Neal]

Comment on attachment 203841 [details] [diff] [review]
Allow more than just email address as a minimum entry requirement

a=me for SM1.0b for SM only part of patch, one more needed!

Comment 16

[[e-mail]]

Comment on attachment 203841 [details] [diff] [review]
Allow more than just email address as a minimum entry requirement

a=me for SeaMonkey

Comment 17

[Mark Banner (:standard8)]

SeaMonkey only parts of the patch checked into the 1.8 branch in time for SeaMonkey 1.0b.

Comment 18

[Mark Banner (:standard8)]

(In reply to comment #17)
> SeaMonkey only parts of the patch checked into the 1.8 branch in time for
> SeaMonkey 1.0b.
> 
Also now checked into the 1.8.0 branch.

Comment 19

[Mark Banner (:standard8)]

Adding dependence to bug 64305 as the patch on that bug is needed if this is implemented on 1.8.1 branch for Thunderbird.

Comment 20

[Mark Banner (:standard8)]

Comment on attachment 203841 [details] [diff] [review]
Allow more than just email address as a minimum entry requirement

Requesting approval for 1.8.1 branch (Thunderbird-only parts, SeaMonkey parts already in). This patch allows a better specification of a minimum entry requirement for address book cards. 
This patch also needs bug 64305 and bug 307056 to be accepted for branch. We'll want this patch for trunk synchronisation for birthday & anniversary fields if we implement them for 2.0.

Comment 21

[Mark Banner (:standard8)]

Thunderbird part checked into branch.

Comment 22

[Marcia Knous [:marcia]]

verified fixed on the 1.8 branch using the Tbird cand build, version 2.0.0.0 (20070326). I used Win Vista for verification. I followed the STR on the initial report, and I am able to save a new card that is missing an email address. adding branch verified keyword.