Closed Bug 451370 Opened 13 years ago Closed 13 years ago

Stop using nsIAbAddressCollecter::GetCardFromAttribute in msgHdrViewOverlay.js

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

(Reporter: standard8, Assigned: jcranmer)

References

Details

Attachments

(1 file)

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.
Blocks: 58769
Attached patch Patch, version 1Splinter Review
Here's a patch which removes it from JS.
Assignee: mail → Pidgeot18
Status: NEW → ASSIGNED
Attachment #335173 - Flags: review?(bugzilla)
Keywords: helpwanted
No longer blocks: 450149
Depends on: 450149
No longer depends on: 450149
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+
Attachment #335173 - Flags: superreview?(kairo)
Attachment #335173 - Flags: superreview?(kairo) → superreview?(dmose)
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 on attachment 335173 [details] [diff] [review]
Patch, version 1

Wouldn't you be better off fixing nsAbAddressCollecter.cpp [sic]?
(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 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+
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
Closed: 13 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.