Stop using nsIAbAddressCollecter::GetCardFromAttribute in msgHdrViewOverlay.js

RESOLVED FIXED in seamonkey2.0a1

Status

SeaMonkey
MailNews: Message Display
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: standard8, Assigned: jcranmer)

Tracking

Trunk
seamonkey2.0a1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
Once bug 450149 is completed there will be a new method on nsIAbDirectory called cardForProperty.

There is one use of nsIAbAddressCollecter::GetCardFromAttribute in msgHdrViewOverlay.js (setFromBuddyIcon), this should be switched over to use the new nsIAbDirectory::cardForProperty.

I would recommend changing it to either cycle round all the address books or just getting the collected address book uri from prefs and getting that directory.

Either way, we need nsIAbAddressCollector::GetCardFromAttribute to die.
(Reporter)

Updated

10 years ago
Blocks: 58769
(Assignee)

Comment 1

10 years ago
Created attachment 335173 [details] [diff] [review]
Patch, version 1

Here's a patch which removes it from JS.
Assignee: mail → Pidgeot18
Status: NEW → ASSIGNED
Attachment #335173 - Flags: review?(bugzilla)
(Assignee)

Updated

10 years ago
Keywords: helpwanted
(Assignee)

Updated

10 years ago
No longer blocks: 450149
Depends on: 450149
(Reporter)

Updated

10 years ago
No longer depends on: 450149
(Reporter)

Comment 2

10 years ago
Comment on attachment 335173 [details] [diff] [review]
Patch, version 1

+       var directory = directories.getNext();
+                                  .QueryInterface(Components.interfaces.nsIAbDirectory);

You don't want the ; on the first line here...

r=me with that fixed.
Attachment #335173 - Flags: review?(bugzilla) → review+
(Assignee)

Updated

10 years ago
Attachment #335173 - Flags: superreview?(kairo)
(Assignee)

Updated

10 years ago
Attachment #335173 - Flags: superreview?(kairo) → superreview?(dmose)
(Reporter)

Comment 3

10 years ago
Comment on attachment 335173 [details] [diff] [review]
Patch, version 1

Actually, this is a SM specific file, so Neil should do this...
Attachment #335173 - Flags: superreview?(dmose) → superreview?(neil)

Comment 4

10 years ago
Comment on attachment 335173 [details] [diff] [review]
Patch, version 1

Wouldn't you be better off fixing nsAbAddressCollecter.cpp [sic]?
(Reporter)

Comment 5

10 years ago
(In reply to comment #4)
> (From update of attachment 335173 [details] [diff] [review])
> Wouldn't you be better off fixing nsAbAddressCollecter.cpp [sic]?

From my perspective nsAbAddressCollecter is for collecting addresses, not searching through address books.

Maybe we should have a central function somewhere (for going through all address books), but IMHO nsAddressCollecter isn't the place, and its something we probably want to re-examine once we're at the stage where we can remove rdf from the address book - as that will define how the new structure will work.

Comment 6

10 years ago
Comment on attachment 335173 [details] [diff] [review]
Patch, version 1

>      if (myScreenName && card && card.setProperty("_AimScreenName")) {
Isn't this supposed to say card.getProperty("_AimScreenName")?
Attachment #335173 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 7

10 years ago
Checked in, changeset 2b8c8bbe40ca.

(In reply to comment #6)
> (From update of attachment 335173 [details] [diff] [review])
> >      if (myScreenName && card && card.setProperty("_AimScreenName")) {
> Isn't this supposed to say card.getProperty("_AimScreenName")?

Looks like it's a regression from bug 413260. There's a bit more to the code that needs to be changed, but since the path is only executed if you have a certain hidden pref, I'll file another bug on the issue.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(In reply to comment #7)
> I'll file another bug on the issue.

I filed bug 454073 ;->
Target Milestone: --- → seamonkey2.0alpha
You need to log in before you can comment on or make changes to this bug.