183632 - Improve error dialog from Address Selection for card without email address

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

Description

[laurel]

Using dec4 trunk

When you use the Address Selection window ("address picker") and select a card
that happens to have a blank email address, once you confirm the picker you see
an alert dialog which tells you to remove the card(s) without email address. 
The alert uses text indicating it will provide you a list of these cards, but it
doesn't; it provides an empty list (with delimiters/separators) no matter how
many empty email cards you've selected and whether or not they appear in a
picked list of valid email cards.

Side issue:  this alert appears at slightly different points PC vs. Mac.

Steps: 
1.  Enter a card in your address book which has a First, Last and display name,
but an empty email address field.
2.  Open a message composition window, click Address button to get the address
picker. Select the card with blank email field and click To:, CC: or Bcc:
3.  Confirm OK to the picker and you should see an alert which states:
    "Please remove the following cards with no email address::,:..."

Expected:  list of cards picked which need to be removed.

Comment 1

[laurel]

Attachment: Alert screenshot

Comment 2

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

mass re-assign.

Comment 3

[Mark Banner (:standard8)]

Attachment: Patch to correct dialog and get it working again

This patch fixes the dialog display and gets it working again (I think bug
254047) broke it. When I tested it was originally coming up with no dialog, but
a js error saying "text" wasn't defined.

I have also changed the alert function to use the global prompt service as it
says we should.

Comment 4

[Mark Banner (:standard8)]

Nominating for blocking thunderbird (aviary) as if someone has selected a card
with an empty email on the selected addresses dialog and then clicks ok nothing
will happen.

Patch is attached ready for reviews.

Comment 5

[[e-mail]]

Comment on attachment 167831 [details] [diff] [review]
Patch to correct dialog and get it working again

The bustage was caused my fix to bug 254047, sorry :-[
Strangely, with or without the patch, I don't see a display name on cards
without an email address, it just shows "To:" - if this isn't just my build I'd
prefer an alert when you try to add the card (probably by testing the result of
GenerateAddressFromCard).

Comment 6

[Mark Banner (:standard8)]

Attachment: Revised patch to check on addition of card to list.

Revised patch to check on addition of the card to the list that the primary
email address is empty.

Comment 7

[Mark Banner (:standard8)]

Removing blocking nomination flag as that seems to have passed now.

Comment 8

[[e-mail]]

Comment on attachment 167872 [details] [diff] [review]
Revised patch to check on addition of card to list.

I was worried what would happen if typed a really malformed address such as (@)
into the compose window. As it happens the existing dialog doesn't handle that
very well at all. However your new string doesn't sound very user friendly, how
about "A card could not be added because it did not have a primary email
address." or "You cannot add a card that has no primary email address" or maybe
you can come up with a better idea.

Comment 9

[Mark Banner (:standard8)]

Attachment: Revised error text.

Revised patch to give clearer error message as suggested by Neil. Re-using
Neil's r from the previous version.

Comment 10

[Mark Banner (:standard8)]

cbiesinger suggests notes that we should have a title on this dialog. I'll
attach a new patch in a day or so.

Comment 11

[Mark Banner (:standard8)]

Attachment: Add title to dialog

At the request of cbiesinger, added title to the error dialog. cbiesinger
suggested I get this r + sr'd again in case you don't like the string. Sorry if
you wouldn't normally expect that.

Comment 12

[David :Bienvenu]

Comment on attachment 169456 [details] [diff] [review]
Add title to dialog

Whatever Neil thinks is fine with me - a title like "Unable to Add Cards" might
be more informative, but I have no idea what the UI convention is. The title
should probably just be what the operation is, so "Add Cards" is fine with me.

Comment 13

[[e-mail]]

Comment on attachment 169456 [details] [diff] [review]
Add title to dialog

"Select Addresses", "Add Card" and and "Cannot Add Card" are all acceptable.

Comment 14

[Mark Banner (:standard8)]

Attachment: Patch for checkin

Changed title to "Cannot Add Card" as per one of Neil's suggestions. Reusing
Neil's r and David's sr for checkin.

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in mailnews/addrbook/resources/content/abSelectAddressesDialog.js;
/cvsroot/mozilla/mailnews/addrbook/resources/content/abSelectAddressesDialog.js,v
 <--  abSelectAddressesDialog.js
new revision: 1.39; previous revision: 1.38
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.31; previous revision: 1.30
done

Comment 16

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

I now see "You cannot add a card that has no primary email address", so verified
FIXED on Windows XP SeaMonkey trunk build 2005-07-10-05.