Remove presence management

[turt2live]

Partner PR

riot-web: element-hq/element-web#5881

Fixes

This effectively "fixes" element-hq/element-web#5727

Description

The feature is incredibly buggy and doesn't work as expected due to server behaviour and client interaction. One of the major problems is the constantly confused presence state - this is caused by the mobile apps conflicting on the state of the web app, causing it to consider the user offline or online (and rarely away) depending on how riot-android/ios is behaving at the time.

This reverts two PRs:

• MemberPresenceAvatar: fix null references #1620
• Implement user-controlled presence #1482

The changes to the context menu positioning were not reverted as they are useful outside of presence management.

[ara4n]

I'd much rather that we fixed the presence support in Matrix as a whole rather than backed this out, especially given it's behind a feature flag anyway. Presence in Matrix has unfortunately had a pretty chequered history and is currently ownerless (if it wasn't obvious), hence the current mess.

There are some speculative notes over at https://github.com/matrix-org/matrix-doc/blob/master/drafts/general_api.rst#presence-api-draft on how presence might be updated in the v2 CS API, but in practice these never were incorporated into the actual spec, which remains very ambiguous. (These in turn were based on some old and sort-of lost etherpad notes that I and Kegan wrote around Christmas 2015).

However, the problem in general is that we never got agreed on whether presence is intended to track when a user was active on Matrix... or what their status is. When it was first introduced in v1, the two things got conflated together (as well as 'away' and 'unavailable' statuses being somehow mixed up too). We intended to fix this in v2 (as per the draft above) but it never got incorporated.

In the end, it feels like there are basically three ways of handling custom presence; either:

1. we multiplex together the idea of activity (online, idle & offline status values) and custom user status (DND, asleep, etc) - and treat custom user status as 'online'. This is roughly what we do today, but it's a mess as we haven't defined how to merge together statuses across multiple clients (let alone how to merge custom clients)

2. we store status as a totally separate parameter in the presence API to the activity status. This doesn't exist today, but is probably the simplest solution. This risks a nasty duplication of functionality between presence & custom profiles though (see below).

3. we store status in a custom profile API rather than presence. This might cause problems if the visibility semantics of custom profiles (when they exist) don't match those of presence - but then it'd be rather nice if custom profiles were flexible enough to support that visibility.

Other stuff which should probably be considered includes:

• i18n support
• a mix of predefined status states (e.g. DND, AFK, etc) and completely custom freeform ones "I am currently washing my hair"

So, I guess the 'right' way to fix this using option 2 would be:

• Propose a draft to the /presence API to split user status from activity. The good news is that the activity field (online/idle/offline) and semantics stays the same.
• Implement it in Synapse
• Change this PR to test it in riot/web
• PR against the spec to fix it.

(Documenting this here whilst I'm thinking about it, rather than a specific request to @turt2live :)

[turt2live]

Fixing presence would certainly be the way to go, although it being ownerless does mean that this feature continues to be broken. I'd still recommend backing out the PR as the changes are fairly minor and can be re-implemented with ease. Another option might be to remove teh labs flag and leave the presence avatar stuff in the codebase, just deactivated. Part of this is also because it interacts badly with matrix.org.

I agree that the spec should allow for custom statuses, however they should probably be linked to some kind of generalized status (online, away, dnd, appear offline). This would mean that clients can support custom presence if they want to, or fall back to just showing someone as "away" or "online" or whatever.

The usability side of things is a bit interesting. Discord for instance can advertise which game you're playing while saying you're online. However they only let you control your status (online/away/dnd/offline) and whether or not you want the game you're playing advertised.

Telegram on the other hand doesn't seem to let you have a status. They track people by "last seen". The visibility of this is configurable per-user, but no one ever knows if you're online, offline, away, etc - just that you were last seen an hour ago.

Then there's XMPP clients which let you do all kinds of things with your presence. I don't think I've seen a client which allows custom presence ever do this in a nice way, but if Riot can figure it out then that'd be awesome :)

tldr: Supporting a status with custom message would be my vote as it can be interpretted by less capable clients.

[ara4n]

I agree that the spec should allow for custom statuses, however they should probably be linked to some kind of generalized status (online, away, dnd, appear offline).

I think this is a mistake (and would propagate the problems in the current API). To be clear, I am proposing entirely decoupling presence information to do with user activity (i.e. "online", "offline", "online but idle for $N seconds") from generalised user status (e.g. "DND", "Asleep", "Out of Office").

Actually looking at the current API, it seems we are closer to Option 2 than I thought - the API can be used in this shape anyway:

PUT /_matrix/client/r0/presence/%40alice%3Aexample.com/status HTTP/1.1
Content-Type: application/json

{
  "presence": "online",
  "status_msg": "I am here."
}

Where presence is either online, offline or unavailable (which means that the presence is unknown, rather than the user is away/dnd/unavailable. i have no idea why it's called unavailable rather than unknown).

status_msg can then be used for whatever freeform text additional custom state, which is good enough for now. We could also add a status field in future for "m.away" or "m.dnd" style activity states.

So in practice, I think all this needs for a first cut (other than matrix.org enabling presence) is for synapse to make status_msg sticky and not overwrite it every time a client sets its presence, unless a new status_msg is explicitly set?

If so, I'd be very happy to make the necessary changes to the spec & synapse to fix this, so we don't have to back this out.

[turt2live]

This presence stuff doesn't handle status messages anyways (in fact, it ignores them). The concern is that mobile clients can confuse the status in riot-web with no real distinction as to what should be the user's presence.

The easy way is to consider the "most online" status (online > away > offline) the current status, however this causes problems. Annecdotatly this doesn't work because riot-android (at least) sends online and offline statuses depending on whether or not it is syncing, and (I think?) whether or not the app is open. This is particularly noticable on my account where the app often gets killed by android due to resource usage.

Decoupling syncing from presence would probably be a way to go, as that doesn't seem to be making the situation any better. Tangently related is appservice presence (https://github.com/matrix-org/synapse/issues/2385). Presence currently expires - having it not expire and instead leaving it to the client to determine may be a better approach. Making multiple clients interact nicely with each other is a bit of an uphill battle though.

[ara4n]

I think we are still talking cross purposes.

• the fact that /sync sets presence activity state is very much a feature rather than a bug (although you can use the set_presence parameter to do a /sync without appearing online)
• the problem is that the “presence” field should be used strictly for user activity (online/offline and possibly idle, although idleness (last_active) is calculated serverside), and not for random status messages like “away” or “asleep” or “dnd”, otherwise they get overridden and don’t merge sensibly on the server. the spec is wrong.
• so i think the fix here is to juse use status_msg to track these human status messages - and perhaps in future add a status field for m.dnd and m.away style semantic status types.

tl;dr: use status_msg to track away state for now (or other custom statuses) and this feature should just work as intended.

[jfrederickson]

@ara4n Since it's relevant here and it probably got buried under the mountain of synapse issue reports, it's worth noting that synapse also currently appears to stomp over status_msg when your presence changes. (This is matrix-org/synapse#2245.)

[turt2live]

I'm going to go ahead and close this as it seemingly has no chance of making it in and I'm not exactly looking to maintain this kind of PR.

Hopefully someone can pick up where I left off and fix presence.

[ara4n]

hm, i don't think I'd quite realised that this was actively breaking presence (as per element-hq/element-web#5727 and element-hq/element-web#6498), even when the labs feature was disabled?

[turt2live]

Yes, the problem in particular is 6cd0773#diff-eca58d07f024b6e9424f8d1c3ace3771R96

It's the only reason I've been so adamant to rip out the presence stuff I originally put in.

[ara4n]

okay, i must have been smoking crack first time through this conversation - sorry. are you able to fix conflicts or shall I do a new PR?

[turt2live]

This worries me, but I'll take a look and see if I can get it back into shape.

[turt2live]

@ara4n Both sides (riot is element-hq/element-web#5881 ) updated and lightly tested.

[ara4n]

thanks! sorry for being a crank on this previously :(