This is a TODO, which should be looked at after we have OTR initially working.

With the old code that we're porting to TB, the conversation window will show a mix of messages exchanged with the buddy, and OTR status messages, such as starting encrypted chat, ending it, verifying peer etc.

Sometimes the first message is sent without encryption, then an OTR session is automatically started, and the window adds information "private conversation started". That means, messages following will be encrypted, but the previous message wasn't.

I think it should be made more obvious, which individual messages were encrypted, and which weren't.

I suspect this would be an improvement over using system messages!

One thing worth noting is that this would need to be done in a way that allows message styles to be involved.

I can do some mock-ups for this, but I'd need a quick overview on how we currently manage the message section and styling.
Patrick, would you be able to point me towards the right file and write down a quick overview of how that area works?

We actually have decent-ish docs on how the message styles work: https://developer.mozilla.org/en-US/docs/Mozilla/Chat_Core/Message_Styles -- I'm not 100% sure this is what you're looking for, but it should be a good start at least!

The theme files live under https://searchfox.org/comm-central/source/mail/components/im/messages

It's time to tackle also this outstanding OTR issue.
I personally think that the chat UI needs some love and well deserved UI polish, so we could use the occasion to start reorganizing elements and optimize the view for the default style.

In this mock-up proposal, I introduced the following changes:

• Align the message time to the left to gain more horizontal space for the message.
• Use our brand colours for the chat names.
• Use the left space to include smart icons with tooltips indicating the status of a message (encrypted, not-encrypted, not-logged, etc.).

Thoughts?

(In reply to Alessandro Castellani (:aleca) from comment #4)

I personally think that the chat UI needs some love and well deserved UI polish, so we could use the occasion to start reorganizing elements and optimize the view for the default style.

Frankly I find the default style unusable. It has too much whitespace in some places, but not enough separation between participants / messages. Florian and I have always encouraged people to use the Bubbles theme. I don't remember why the Thunderbird theme was created to be honest. I'd encourage looking at the other themes to exist while updating this. (It also styles Twitter differently, which really irks me and goes against one of the main tenants of the chat code.)

In this mock-up proposal, I introduced the following changes:

• Align the message time to the left to gain more horizontal space for the message.

This looks a lot nicer, I wish we could not clip people's names though, it looks very unfinished.

• Use our brand colours for the chat names.

Just an FYI that the colors of the participants in a multi-user chat are auto-generated based on name. In a one-on-one chat they can use those same colors (I think?) or be hard-coded. The message styles code also supports multiple "variants" of the same theme (e.g. Bubbles has Red-and-Blue, Red-and-Green, etc.)

• Use the left space to include smart icons with tooltips indicating the status of a message (encrypted, not-encrypted, not-logged, etc.).

The left space is a bit weird since you can:

• End up with a ton of extra whitespace if the same person sends many messages (or one really long one, I guess).
• Not end up with enough space if there's a lot of interleaved participants.

I think including icons with tooltips somewhere is a reasonable way to do it. The chat code also has a way to dynamically implement "actions" (look for code that uses getActions()), those are currently down via context menus, not sure if any of that would be useful to include here too. Might get too crowded.

Thank you both for your feedback and suggestions, I'm uploading another mock-up to address everything.
I'd like to not go too much overboard on this bug and keep the main focus on "showing the encryption status of a message" while improving a little bit the default theme.

The left space is a bit weird since you can:
-End up with a ton of extra whitespace if the same person sends many messages (or one really long one, I guess).

I'm not concerned about this as it's a pretty normal outcome in every chat client adopting this type of layout, and it's not wasted white space as it's necessary to visually separate the recipients.

• Not end up with enough space if there's a lot of interleaved participants.

I'm not quite sure I understood this. Could you show me a screenshot of this scenario?

For the colours, there is a bug to choose better colours.

Cool, I won't tweak them and leave it for that bug.

Moving the recipient name above on its own line solve various problems of visual separation and spacing, good call Richard.

Regarding the context menu, we can take a page from pretty much every other IM clients and show some context buttons on hover.

The left area would be dedicated to showing the time and any extra icons/states specific to that message.

Having either a closed padlock or an open padlock next to each message seems ok.

When looking briefly, the icons don't differ much, "there's a litte grey thing" for both. I wonder if they could be slightly changed so the difference is easier to see.

FX has a icon with a red stroke through the padlock.

I removed the red line because it was getting too much attention.
What if instead we simply don't show anything and we only show the lock when a message is encrypted?
We remove redundancy and clean up that area when dealing with a default status.

(In reply to Alessandro Castellani (:aleca) from comment #8)

Moving the recipient name above on its own line solve various problems of visual separation and spacing, good call Richard.

I agree! That solves a lot of my complaints about the whitespace not being enough of a separator.

Overall I think that looks pretty good! A lock seems reasonable to me, if that is consistent with other locations in our application. If we think we're going to want more than one icon it might make sense to put it below the date instead of next to it, but it looks nice as is now! Good work!

I updated the style of the chat thunderbird theme and implemented the new isEncrypted attribute.

I can't seem to find the proper location where an OTR message gets sent, and how to add that flag.
Kai, could you please point me to the right location?

I'm pinging Patrick and Richard to be sure I haven't caused any regressions in styling or readability.

Especially with the smaller font size the smilies are more off centre. Maybe, when you're on changing things in chat code you could also change https://searchfox.org/comm-central/source/chat/themes/conv.css#11 from text-bottom to middle. What do you think?

(In reply to Alessandro Castellani (:aleca) from comment #14)

I updated the style of the chat thunderbird theme and implemented the new isEncrypted attribute.

I can't seem to find the proper location where an OTR message gets sent, and how to add that flag.
Kai, could you please point me to the right location?

There might not be a very simple answer to this question, because there are a lot of callbacks involved.
I remember I had the same problem when I had to implement the logging (or not logging) of encrypted messages.

Could you please look at the commit for bug 1536104 to see if that helps you?

(In reply to Richard Marti (:Paenglab) from comment #15)

Why do you make the normal text smaller. I saw a lot of complaints that the
standard size is already too small. I think, this should stay on normal size
and the other sizes grow one thenth.

Really? That's weird as for me the default font size is incredibly big.
I'm on a HiDPI monitor with large font scaling, and it seems as only this section of Thunderbird gets doubled in size.
I'll revert the sizes to what they were and deal with this on a dedicated bug.

I fixed the emoji alignment.

(In reply to Kai Engert (:KaiE:) from comment #17)

Could you please look at the commit for bug 1536104 to see if that helps you?

Thanks, I'll check it out.

I did it!
Since Kai already set the encrypted attribute in the imConversation, I was able to easily check for it to show/hide the encrypted lock.

Fixed a tiny padding issue.
If this gets approved, I will upload a follow up to add the lock indicator to all the other message styles.

I applied this locally to test it. I merged with the changes from bug 1549938.

The behavior looks good to me in general. Omitting the padlock for unencrypted messages seems ok to me.

I've tested the scenario described in bug 1519796 comment 18. The message is shown with the padlock, although it wasn't actually sent out with encryption. (In particular, it wasn't sent out at all.) However, the message is promised to NOT be sent without protection, so the behavior seems acceptable to me (the system messages explain that the message has not been sent (yet)). The bug here is rather that we display such queued/pending messages normally. Maybe a separate bug should be filed to request "show pending/queued outgoing messages differently".

(In reply to Kai Engert (:KaiE:) from comment #22)

The bug here is rather that we display such queued/pending messages normally. Maybe a separate bug should be filed to request "show pending/queued outgoing messages differently".

I think this is something we've wanted, just never implemented. Sounds separate to me!

Tanks for the review.

Generating this string bundle for every encrypted message sounds like it will have poor performance.

How would you approach this issue? Do we want to have a string at all for that icon?

These changes look unrelated to this bug? I'm a bit confused at what this is doing here.

Sorry, those slipped in by mistake.

(In reply to Alessandro Castellani (:aleca) from comment #26)

Tanks for the review.

Generating this string bundle for every encrypted message sounds like it will have poor performance.

How would you approach this issue? Do we want to have a string at all for that icon?

Something like this: https://searchfox.org/comm-central/rev/2131bede09ff0e7273bc77f669150491cd0d7685/chat/components/src/imConversations.jsm#17-19

Thanks Florian for the heads up, I'll remove the whitespace.

Should there be a readme file next to those files in the same directory, that explains that rule?

Fixed everything.
The bubbles theme uses its own minus and plus PNGs in within the theme's folder.

I'm not sure how the Content.html is hooked up, but is it possible to just use Fluent? I.e. just add data-l10n-id?

Any chance of reviewing/landing this before the next beta?
I'm coordinating a user testing session of the OTR implementation and having this improvement would be great.

Updated.

Just to confirm, the lock works for me.
I also launched a try run to see if I busted something: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b5e906bff305055eed77f12f4cc2edd6c9dca4c5

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/911aa4dd69ad
Improve the default message style UI and show the OTR encryption status of individual messages. r=clokep

This is a follow up patch taking care of all the other themes.

Here's a screenshot with all the chat styles with encrypted messages.
They're not "perfect", but as a first implementation I think this is acceptable.
We can later improve those styles in dedicated bugs.

(In reply to Alessandro Castellani (:aleca) from comment #42)

Thanks for the screenshots. The encrypted/secure nature of the message seems attached to the name of the sender, so for message themes that group messages together, I think it would make more sense to show the lock icon before the name of the sender, rather than before every messages.

nit: Spacing issues (only relevant if you keep the icons as they are) : there's not enough horizontal padding on both sides of the lock icons on the screenshot of the Bubbles theme. On the third theme in your screenshot, the vertical alignment of the lock icon in the first message of a message group seems a bit off.

Thanks Florian for the feedback.
I'll upload a follow up patch to address all your valid points.

Patch updated to implement the icon as pseudo element inline the sender name.
I also removed the string as title tag since that wasn't working, and implemented the OTR Status: Message encrypted row in the chat buddy tooltip, in case the message is encrypted.

Thanks for the detailed review, I fixed all the issues you pointed out.

I'm not sure when this will be shown to users. Are we currently showing tooltips when hovering the message sender for every message?

Yes, we're showing a tooltip with the user info and profile image when the mouse hovers over the sender's name.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/3fa5a7d06f89
Implement the encrypted lock icon for all the chat message styles. r=florian

We should likely uplift that additional patch so all of this bug is fixed in 78.

77.0b2: https://hg.mozilla.org/releases/comm-beta/rev/37ffaf87bf83f9a413f8eac4b5537a04f6024db7