Look into merging "duplicated" getCardForAddress()

NEW
Assigned to

Status

MailNews Core
Address Book
--
trivial
9 years ago
5 years ago

People

(Reporter: sgautherie, Assigned: aceman)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

9 years ago
(Noticed while working on bug 309057 comment 67.)
(Assignee)

Comment 1

6 years ago
What is the proped function you have seen duplicated?
The summary contains getCardForAddress but the mxr link has cardForEmailAddress.
(Reporter)

Comment 2

6 years ago
The bug is correct as filed:
*(both) mailWindowOverlay.js has code that looks rather similar to getCardForAddress().
 *Could getCardForAddress() be (adapted and) reused?
*getCardForAddress() is coded in (both) msgHdrViewOverlay.js.
 *Could it be shared in /mailnews?
 */mail file seems to have diverged in the meantime, but the 3 others can probably be resync'ed...
(Assignee)

Comment 3

6 years ago
I still do not see 'cardForEmailAddress' in your reply. It is in the URL field filled by you.
(Reporter)

Comment 4

6 years ago
(In reply to :aceman from comment #3)
> I still do not see 'cardForEmailAddress' in your reply. It is in the URL
> field filled by you.

? cardForEmailAddress() call is (part of) the common code at all these places!
(Assignee)

Comment 5

6 years ago
I see what you mean now. There are some very similar loops, in some places a whole function called "getCardForAddress" that call "cardForEmailAddress". You want to merge those JS loops, not "cardForEmailAddress" (there seems to be only one and it is in C++).

But I do not see any shared file imported by all the files that would be touched here. It seems it would need to create some new AButils.js file that could be imported in all of them.
Mconley, what do you think?
Assignee: nobody → acelists
(Assignee)

Comment 6

5 years ago
mconley, will all this become obsolete when you do the new addressbook?
(In reply to :aceman from comment #6)
> mconley, will all this become obsolete when you do the new addressbook?

Yes, likely.

In the meantime, this bug can probably be patched without tooooo much fuss.

-Mike
(Assignee)

Comment 8

5 years ago
(In reply to Mike Conley (:mconley) from comment #7)
> In the meantime, this bug can probably be patched without tooooo much fuss.

Ok, but you didn't reply as to how :)
(In reply to :aceman from comment #5)
> But I do not see any shared file imported by all the files that would be
> touched here. It seems it would need to create some new AButils.js file that
> could be imported in all of them.
> Mconley, what do you think?

I'm not sure what SeaMonkey needs or uses for their address book, unfortunately. I do know that mail/ has abCommon.js where we stash all of our utility functions - this is likely where your code would go, if I'm understanding the bug correctly.

Not sure how the SM folks would want to proceed.

-Mike
(Assignee)

Comment 10

5 years ago
They do have an abCommon.js too.
Actually a missed that bit that this is mostly about SM.
Only if the " *Could it be shared in /mailnews?" part is to be implemented then it get relevant for TB. But I do not see any good file in /mailnews either.
You need to log in before you can comment on or make changes to this bug.