progval
Track display names and include them in WHOIS and WHO
I'm using this locally and it seems to work fine. If display names are ever used to derive nicks this will need more work (in particular, send NICK when a user's display name changes), but this suffices for now.
The existing TODOs said to use the "most common" display name, but since display names are per-user rather than per-channel or per-message, we can just pick an arbitrary member record and get the display name from there; the only time these will be inconsistent is if the user has just changed their name and the server is still in the process of sending m.room.member events to propagate it to all the channels.
but since display names are per-user rather than per-channel or per-message
Display names are per-room. The way they are transmitted is through each room's latest m.room.member state event for the given user.
Display names are per-room. The way they are transmitted is through each room's latest
m.room.memberstate event for the given user.
According to the spec, they're per-user:
- The API for fetching a user's display name has no room scope; the only argument is the userid
- 9.2.1 "Events on Change of Profile Information" says that "an
m.room.memberevent (with ajoinmembership) is sent to every room the user is a member of, to update thedisplaynameandavatar_url."
There's nothing physically preventing a homeserver from sending m.room.member events to different rooms offering different display names for the same user, but that's not in spec, and if a server does that I think it's entirely valid to just pick one of those arbitrarily, although probably the most correct way to handle that sort of misbehaviour is to treat the m.presence (which the server should also send) as canonical.
I see, that's for fetching display names from the user's profile, which is global.
However, this PR uses room state rather than the profile to fetch display names, which may differ from each other and/or the profile.
Looking into it more (especially matrix-org/matrix-spec#103 which has a lot of discussion on this topic), it looks like the current state of affairs is:
- clients can set their profile information per-room by manually crafting an
m.room.memberstate event and sending it to the server, which the spec discourages but does not outright forbid - some clients, including Element, expose this capability to the user
- it's unclear if the server is actually required to retain this per-room information, although Synapse does
- setting your profile information "normally" overwrites your per-room information in all rooms that you are in, and this behaviour is required by the spec (9.2.1 again)
Based on this I'm pretty comfortable saying that per-room display names are not well supported, and are more of a side effect of how most servers handle m.room.member events than an intentional feature. There have been various attempts over the years to make it intentional (e.g. matrix-org/matrix-spec#323, matrix-org/matrix-spec-proposals#1302, matrix-org/matrix-spec-proposals#3189) but they're all either stalled out or blocked on more fundamental changes to the protocol.
All that said, we have a few other options here, if we don't trust room state to be consistent; the easiest and most spirit-of-the-law is, I think, to simply call the /displayname endpoint and use the user's profile displayname in WHOIS, rather than risking skew between different room states, and I'm happy to rework the PR to do that if you'd rather take that approach.
All that said, we have a few other options here, if we don't trust room state to be consistent; the easiest and most spirit-of-the-law is, I think, to simply call the
/displaynameendpoint and use the user's profile displayname in WHOIS, rather than risking skew between different room states, and I'm happy to rework the PR to do that if you'd rather take that approach.
Sounds good
(but keep the per-room display-name in the @draft/display-name message tag)
Will do, not planning to touch that code, just WHO/WHOIS. I'll also keep the commit that properly retains updates to per-channel display names even though it won't be load-bearing for the rest of the PR after this.