Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC1862: Presence Capabilities #1862

Open
wants to merge 21 commits into
base: old_master
Choose a base branch
from
Open

Conversation

Half-Shot
Copy link
Contributor

@Half-Shot Half-Shot commented Feb 7, 2019

@Half-Shot Half-Shot added the proposal A matrix spec change proposal label Feb 7, 2019
Co-Authored-By: Half-Shot <will@half-shot.uk>
@richvdh

This comment has been minimized.

proposals/1862-presence-capabilites.md Outdated Show resolved Hide resolved
proposals/1862-presence-capabilites.md Outdated Show resolved Hide resolved
@matrix-org matrix-org deleted a comment from ara4n Feb 10, 2019
@turt2live
Copy link
Member

Seems as though this proposal has become a bit of a cold case. In an effort to incite commentary...

@mscbot fcp merge

@mscbot
Copy link
Collaborator

mscbot commented Mar 22, 2019

This FCP proposal has been cancelled by #1862 (comment).

Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people:

Concerns:

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge labels Mar 22, 2019
@Half-Shot
Copy link
Contributor Author

@turt2live This is now a thing at matrix-org/synapse#5666

@turt2live

This comment has been minimized.

@Half-Shot
Copy link
Contributor Author

I've added a simple riot implementation too

@turt2live

This comment has been minimized.

An omission of `m.presence` would default both sending and receiving to true. An omission of either child flag
would also default that flag to true.

When `send_enabled` is `false`, homeservers should respond to requests to `PUT /_matrix/client/r0/presence/{userId}/status` with `M_FORBIDDEN`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather just silently accept tbh, otherwise I think its going to confuse clients who don't know that disabling presence is is a "thing". If we are going to 403 then we should have a non-generic errcode so that clients can handle that case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively: return a 200 but put some indication in the response.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the current state of the proposal (not passing presence capabilities across federation) I tend to agree that 200 with the response saying "I don't support presence, actually" would be more consistent.

@erikjohnston

This comment has been minimized.

@Half-Shot

This comment has been minimized.

@richvdh

This comment has been minimized.

@Half-Shot

This comment has been minimized.

is changed. Clients SHOULD ensure that they have checked the capailities API before assuming that the parameter
will work.

## Tradeoffs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forking a thread to respond to @richvdh @Half-Shot

Yes, certainly a lot of this could be solved by listening for receipts/messages and using that as a faux presence system.

This seems like the wrong way to solve the problem.

Specifically I was thinking in terms of member lists. If part of the problem is that with presence disabled the member list is weird (avatars greyed out, actively talking people listed last etc), then I think it makes sense to make the member list take into account non-presence info for things like ordering.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the rest of this conversation into the thread:

@erikjohnston said:

To echo @richvdh: I'm not sure this solves what we want it to solve. Presumably this is trying to fix the UX of not knowing if we have accurate presence information, but I don't actually think this solves that as we would still have unknown presence due to other servers.

Would this be better solved with having e.g. riot handle unknown presence better? Not greying out avatars and having the room list order incorporate last time we saw an action from a user I think could go a long way to making this proposal redundant.

@Half-Shot said:

Would this be better solved with having e.g. riot handle unknown presence better? Not greying out avatars and having the room list order incorporate last time we saw an action from a user I think could go a long way to making this proposal redundant.

Yes, certainly a lot of this could be solved by listening for receipts/messages and using that as a faux presence system.

The other half to the proposal is giving servers a way to report that PUT will not be handled by the homeserver, and I think there is still some value in reporting something other than success there.

@richvdh said

Yes, certainly a lot of this could be solved by listening for receipts/messages and using that as a faux presence system.

This seems like the wrong way to solve the problem.

The other half to the proposal is giving servers a way to report that PUT will not be handled by the homeserver, and I think there is still some value in reporting something other than success there

Could you enlarge upon this? What is the value of it?

As per #1862 (comment), I think I have progressed today from "why is this needed at all" to "ok, but this feels like the wrong solution". I feel like we can achieve the same effect with minor tweaks to the APIs instead.

@Half-Shot said:

The other half to the proposal is giving servers a way to report that PUT will not be handled by the homeserver, and I think there is still some value in reporting something other than success there

Could you enlarge upon this? What is the value of it?

Certainly. A few clients have experimented with supporting users setting their own presence status at one point or another, where a user had to hit buttons to change their presence state rather than using sync to do it (which is the point of this endpoint).

If I hit buttons to change my presence state in my matrix client and it happily does so, while the actual request is being ignored by the homeserver, that's a rather poor UX. Certainly if you expect "unavailable" to mean "please don't bother me" and then people bother you because the feature doesn't work, that seems pretty awful.

Which brings me back to the capabilities flag. I'd rather disable features like these in clients where the homeserver doesn't support them by way of flag, but failing that I would like for error codes to be returned when a feature doesn't work so that the client can relay that information back to the user.

@erikjohnston said:

Yes, certainly a lot of this could be solved by listening for receipts/messages and using that as a faux presence system.

This seems like the wrong way to solve the problem.

Specifically I was thinking in terms of member lists. If part of the problem is that with presence disabled the member list is weird (avatars greyed out, actively talking people listed last etc), then I think it makes sense to make the member list take into account non-presence info for things like ordering.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Half-Shot said:

If I hit buttons to change my presence state in my matrix client and it happily does so, while the actual request is being ignored by the homeserver, that's a rather poor UX. Certainly if you expect "unavailable" to mean "please don't bother me" and then people bother you because the feature doesn't work, that seems pretty awful.
Which brings me back to the capabilities flag. I'd rather disable features like these in clients where the homeserver doesn't support them by way of flag, but failing that I would like for error codes to be returned when a feature doesn't work so that the client can relay that information back to the user.

Yes, I agree that there is value in using a capabilities API for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikjohnston said:

Specifically I was thinking in terms of member lists

what exactly are member lists, and how could they be used to solve this problem?

@turt2live turt2live added the kind:feature MSC for not-core and not-maintenance stuff label Apr 20, 2020
@ShadowJonathan

This comment has been minimized.

@richvdh
Copy link
Member

richvdh commented Jan 19, 2021

it's pretty clear that this MSC needs some more attention; for example the thread at #1862 (comment) ends with @Half-Shot agreeing to replace the "should clients show whether users are online" logic with information based on the results of /sync, which doesn't seem to have happened.

meanwhile #1862 (comment) has got a bit confused with conflating the behaviour on the read side and on the send side, possibly with some alternative suggestions about how to proceed?

In any case, it seems we're agreed that capabilities are not the right mechanism for determining how clients should display presence state, so this is back to @Half-Shot to update. It might be best to replace this MSC with alternative proposals which handle the send and read side separately, but I'll leave that to you.

In the meantime I'm going to make an executive decision to

@mscbot fcp cancel

@mscbot mscbot added proposal-in-review and removed disposition-merge proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Jan 19, 2021
@turt2live turt2live added the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.