Closed Bug 1409901 Opened 3 years ago Closed 3 years ago

nicks inside messages should be detected and colored

Categories

(Thunderbird :: Instant Messaging, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 58.0

People

(Reporter: florian, Assigned: florian)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Instantbird detects nicks within IRC messages and colors them. The detected nicks also get nice tooltips and context menus.
Attached patch PatchSplinter Review
Port of the Instantbird code. The new methods are unmodified except for these 2 lines that Instantbird has but Thunderbird doesn't need (yet) in getShowNickModifier:
                if (!buddy.colorStyle)
                  this._setBuddyColor(buddy);

Instantbird has a bunch of optimizations related to the participant list that we should consider porting to TB. I've noticed:
- lazy computation of nick colors,
- when displaying the whole list at once, sorting the nicks first and appending all DOM nodes, rather than doing an insertion sort on the DOM nodes.

There's also code in Instantbird to keep the 'active' status of a nick for 5 minutes if the nick leaves and rejoins. I think this is part of tab completion improvements that aleth did on Instantbird and hasn't ported.

We should do a comparative review of TB's imconversation.xml against IB's conversation.xml
Attachment #8920736 - Flags: review?(clokep)
Assignee: nobody → florian
Status: NEW → ASSIGNED
Comment on attachment 8920736 [details] [diff] [review]
Patch

Review of attachment 8920736 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/im/content/imconversation.xml
@@ +1295,5 @@
> +             // of the nick and aNode is a text node with the text after
> +             // the nick. The text in aNode hasn't been processed yet.
> +             let nick = nickNode.data;
> +             let elt = aNode.ownerDocument.createElement("span");
> +             elt.setAttribute("class", "ib-nick");

Are we OK with this class being ib-nick? (I think we need to be if we want to be able to use themes that are compatible with Instantbird.)
Attachment #8920736 - Flags: review?(clokep) → review+
(In reply to Patrick Cloke [:clokep] from comment #2)
> Comment on attachment 8920736 [details] [diff] [review]
> Patch
> 
> Review of attachment 8920736 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mail/components/im/content/imconversation.xml
> @@ +1295,5 @@
> > +             // of the nick and aNode is a text node with the text after
> > +             // the nick. The text in aNode hasn't been processed yet.
> > +             let nick = nickNode.data;
> > +             let elt = aNode.ownerDocument.createElement("span");
> > +             elt.setAttribute("class", "ib-nick");
> 
> Are we OK with this class being ib-nick?

We need this class name for http://searchfox.org/comm-central/source/chat/content/imtooltip.xml#472

Or if we want to rename it, we need to rename it in imtooltip.xml too.
OK, let's leave it then. No reason to rename things just to rename them.
Pushed by florian@queze.net:
https://hg.mozilla.org/comm-central/rev/2dc8f2e4802d
nicks inside messages should be detected and colored, r=clokep.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
You need to log in before you can comment on or make changes to this bug.