Closed Bug 1779034 Opened 3 months ago Closed 3 months ago

Remove obsolete chat services from SeaMonkey address book

Categories

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

enhancement

Tracking

(seamonkey2.53? affected)

RESOLVED FIXED
Future
Tracking Status
seamonkey2.53 ? affected

People

(Reporter: frg, Assigned: frg)

References

(Blocks 1 open bug)

Details

(Whiteboard: SM2.53.14)

Attachments

(2 files, 1 obsolete file)

AIM and now Google Talk are dead. We still have list in the address book.

Attached patch 1779034-removeaim-25314.patch (obsolete) — Splinter Review

We need to keep some definitions because of shared mailnews code. I also removed the IM button in the ab. It is useless. TB allows to open the internal chat but we only have support for irc and even then I fail to see the value of it. As a follow-up maybe add the matrix nickname to the chat name page.

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: Usmess names in adress book
Testing completed (on m-c, etc.): 2.53.14
Risk to taking this patch (and alternatives if risky): trivial
String changes made by this patch: various removals.

Attachment #9284974 - Flags: review?(iannbugzilla)
Attachment #9284974 - Flags: approval-comm-release?
Comment on attachment 9284974 [details] [diff] [review]
1779034-removeaim-25314.patch

>+++ b/suite/mailnews/components/addrbook/content/abCardViewOverlay.js

>     // Chat section
>-    visible = cvSetNodeWithLabel(data.cvGtalk, zGtalk,
>-                                 card.getProperty("_GoogleTalk"));
>-    visible = cvSetNodeWithLabel(data.cvAIM, zAIM,
>-                                 card.getProperty("_AimScreenName")) || visible;
>     visible = cvSetNodeWithLabel(data.cvYahoo, zYahoo,
>                                  card.getProperty("_Yahoo")) || visible;
The || visible; needs removing from this line (see GoogleTalk line).

Is it worth amending the classic icons to remove the now unused icon(s)?

r/a=me with those fixed / addressed.
Flags: needinfo?(frgrahl)
Attachment #9284974 - Flags: review?(iannbugzilla)
Attachment #9284974 - Flags: review+
Attachment #9284974 - Flags: approval-comm-release?
Attachment #9284974 - Flags: approval-comm-release+
Depends on: 1779319
Flags: needinfo?(frgrahl)

There was an error in the first patch which caused some header breakage. Given that it was just a matter of removing a call to no longer existing setFromBuddyIcon() I am keeping the r/a+ from IanN. Also some globals removed which were only used by setFromBuddyIcon().

I thought about fixing the classic icon strip but think can go in a follow-up because probably lots of css changes. Also maybe we can reuse them for something else later.

Attachment #9284974 - Attachment is obsolete: true
Attachment #9285753 - Flags: review+
Attachment #9285753 - Flags: approval-comm-release+
Attachment #9285753 - Flags: approval-comm-esr60+

Some cleanups var to let conversion and initializing.

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: JJust not so nice code
Testing completed (on m-c, etc.): 2.53.14b1 pre
Risk to taking this patch (and alternatives if risky): trivial
String changes made by this patch: --

Attachment #9285754 - Flags: review?(iannbugzilla)
Attachment #9285754 - Flags: approval-comm-release?
Attachment #9285754 - Flags: approval-comm-esr60?

Comment on attachment 9285754 [details] [diff] [review]
1779034-2-cleanup-25314.patch

[Triage Comment]
Good clean up r/a=me

Attachment #9285754 - Flags: review?(iannbugzilla)
Attachment #9285754 - Flags: review+
Attachment #9285754 - Flags: approval-comm-release?
Attachment #9285754 - Flags: approval-comm-release+
Attachment #9285754 - Flags: approval-comm-esr60?
Attachment #9285754 - Flags: approval-comm-esr60+

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/b14a487f22ed
Remove dead AIM and Google Talk from Address book. r=IanN
https://hg.mozilla.org/comm-central/rev/642810cc383d
Clean up DisplayCardViewPane a bit. r=IanN

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Future
Blocks: 1751312
You need to log in before you can comment on or make changes to this bug.