The default bug view has changed. See this FAQ.

[RFE] when clicking on a mail adress that is already in the adressbook the option should be "edit address book entry", not "add to address book"

RESOLVED FIXED in seamonkey2.13

Status

SeaMonkey
MailNews: Message Display
--
enhancement
RESOLVED FIXED
15 years ago
5 years ago

People

(Reporter: Helge Hielscher, Assigned: Ian Neal)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.13
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
when I click on a mail adress I get 4 options: "add to address book", "compose
mail to", "copy email address" and "create filter".

When I choose "add to address book" it adds a new entry in the address book even
if there is alreday a card for that address.

The better way would be to look up the addressbook first if there is already an
entry and when it is there prompt "edit address book entry" instead or
additional to "add to address book".

Updated

15 years ago
Severity: normal → enhancement

Comment 1

14 years ago
This seems like a sensible RFE to me. Can't find any dupes under a few obvious
searches.

This may even be a bug. It seems obvious if an address is already in the book,
it shouldn't add another one.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: PC → All
Product: Browser → Seamonkey

Updated

12 years ago
Assignee: sspitzer → mail

Comment 2

12 years ago
Agreed.  In fact, I mentioned this in bug 310601 -- you never know 
whether "add to address book" is going to do anything, since there is no 
indication that a person (on the CC list, for example) is already in your 
address book.

Comment 3

11 years ago
*** Bug 262100 has been marked as a duplicate of this bug. ***

Comment 4

11 years ago
Created attachment 229347 [details] [diff] [review]
partial wip against tb 1.5.0.4

This patch is _not_ working: It has all the UI stuff sorted out just fine, the edit address book card dialog opens up as it should, there's just one problem: I need to figure out how to lookup an address book card based on e-mail address and/or display name. Any ideas?

Standard8: Please review the patch modulo this issue.

If and when I can get this to work, I'll make the same changes in seamonkey as well.
Assignee: mail → eyalroz
Status: NEW → ASSIGNED
Attachment #229347 - Flags: review?(bugzilla)
(In reply to comment #4)
> Created an attachment (id=229347) [edit]
> I need to figure out how to lookup an address book card based on e-mail address
> and/or display name. Any ideas?

Sorry for the delay, I've been needing to concentrate on other stuff for a while. I haven't looked at the entire patch yet, I think one way way to do this would be to copy the idea of the address collector (http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsAbAddressCollecter.cpp#132) - though that wouldn't work for LDAP directories. So the better alternative is to copy the quick search method and get the ab out of RDF with a URI supplied which includes the search information as per http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/resources/content/addressbook.js#709 and http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsAbView.cpp#222

I'd also like to see the option for searching multiple address books possibly. 

Comment 6

11 years ago
(In reply to comment #5)

AFAICT all of that code is based on having an addressbook window with a  dirtree and what-not. It mixes UI and backend code in very weird ways I would really not want to get into. Just tell me what the relevant API is (if it exists, of course; otherwise we need a new bug blocking this one).
Comment on attachment 229347 [details] [diff] [review]
partial wip against tb 1.5.0.4

Sorry for not getting back to you much earlier on this

(In reply to comment #6)
> (In reply to comment #5)
> 
> AFAICT all of that code is based on having an addressbook window with a 
> dirtree and what-not. It mixes UI and backend code in very weird ways I would
> really not want to get into. Just tell me what the relevant API is (if it
> exists, of course; otherwise we need a new bug blocking this one).
> 

See Scott's patch on bug 357321 specifically attachment 245415 [details] [diff] [review] and the function allowRemoteContentForSender. That should give you all the information you need.
Attachment #229347 - Flags: review?(bugzilla) → review-

Comment 8

10 years ago
Will not be able to work on this at all, at least for the next month or so; unassigning for now (although it doesn't look like a lot of work).
Assignee: eyalroz → mail
Status: ASSIGNED → NEW
QA Contact: esther
This has been fixed for Thunderbird by bug 450724 attachment 336332 [details] [diff] [review]
Depends on: 450724
Keywords: helpwanted
(Assignee)

Comment 10

5 years ago
Created attachment 630190 [details] [diff] [review]
Add ability to edit/view contacts that already have an addressbook card [Checked in: Comment 12]

This patch ports:
* Front end parts of Bug 450724 - Implement New/Edit Card inline features for message header display.
* The storing headerName parts (but not the "You" string bits) of Bug 481966.
* Front end helper parts of Bug 474721.
* Various other follow-up bugs to the above ones.
Assignee: mail → iann_bugzilla
Attachment #229347 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #630190 - Flags: review?(mnyromyr)
(Assignee)

Updated

5 years ago
Keywords: helpwanted
(Assignee)

Updated

5 years ago
Blocks: 761720
(Assignee)

Updated

5 years ago
Blocks: 761852

Comment 11

5 years ago
Comment on attachment 630190 [details] [diff] [review]
Add ability to edit/view contacts that already have an addressbook card [Checked in: Comment 12]

Just some nits …

>+function AddContact(emailAddressNode)

Changing the function name is a good opportunity for correcting the parameter name to aEmailAddressNode.

>+function EditContact(emailAddressNode)

Idem.

>+  if (emailAddressNode.cardDetails.card) {

Brace on its own line, please.

>+// These are all the items that use a mail-multi-emailHeaderField widget and
>+// therefore may require updating if the address book changes.
>+const gEmailAddressHeaderNames = ["from", "reply-to",
>+                                  "to", "cc", "bcc"];

No need to wrap.

>+  // stash this so that the <mail-multi-emailheaderfield/> binding can

"Stash" should be capitalized here. (Mega minor dragon dinosaur præhistoric nit! *g*)

>+var AddressBookListener = {

Braces on their own line, please; ditto for all the new code following.

>+    OnAddressBookDataChanged(aItem instanceof nsIAbCard ?
>+                             nsIAbListener.directoryItemRemoved :
>+                             nsIAbListener.directoryRemoved,
>+                             aParentDir, aItem);

Maybe indent the two middle lines to highlight that the are parameters of the operator?

>+function UpdateEmailNodeDetails(aEmailAddress, aDocumentNode, aCardDetails)
> {>+  var cardDetails = aCardDetails ? aCardDetails :
>+                                   getCardForEmail(aEmailAddress);

I think you mean:
  var cardDetails = aCardDetails || getCardForEmail(aEmailAddress);

>+  var displayName;
>+  if (condense && cardDetails.card)
>+    displayName = cardDetails.card.displayName;

displayName should be properly initialized to "".

>+  if (displayName) {
>+    aDocumentNode.setAttribute("tooltiptext", aEmailAddress);
>+  } else {
>+    aDocumentNode.removeAttribute("tooltiptext");
>+    displayName = aDocumentNode.getAttribute("fullAddress") ||
>+                  aDocumentNode.getAttribute("displayName");
>+  }

Braces on their own line, please; ditto for all the new code following.

>+      if (aItem instanceof Components.interfaces.nsIAbDirectory) {

Why no constant for Components.interfaces.nsIAbDirectory, but for nsIAbListener/nsIAbCard?

>+          // Just in case we have a bogus parent directory

Just "just".

>+            var cardDetails = { book: aParentDir, card: aItem };

Could use let instead of var here.

>+function setupEmailAddressPopup(aAddressNode)

Why is this function lowercased? TB compat? If not, please use uppercase as well.

>+function getCardForEmail(emailAddress)

Same question here?
Also, worth taking the chance of using aEmailAddress.


r=me with that explained/corrected, especially the braces. ;-)
Attachment #630190 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 12

5 years ago
Comment on attachment 630190 [details] [diff] [review]
Add ability to edit/view contacts that already have an addressbook card [Checked in: Comment 12]

Checked in with points addressed:
http://hg.mozilla.org/comm-central/rev/0d2ec5a8ce0e
Attachment #630190 - Attachment description: Add ability to edit/view contacts that already have an addressbook card → Add ability to edit/view contacts that already have an addressbook card [Checked in: Comment 12]
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.13
(Assignee)

Updated

5 years ago
Blocks: 763284
You need to log in before you can comment on or make changes to this bug.