MSC1804: Advertising capable room versions to clients #1804
Conversation
|
this looks plausible to me fwiw |
We'll still need something like matrix-org/matrix-spec-proposals#1804 to make this work correctly, but this fixes the immediate issue in element-hq/element-web#8154
There was a problem hiding this comment.
Having been persuaded by @turt2live, this looks like a good way to represent room versions supported by the server and works nicely with MSC1753. One minor comment about the proposal.
turt2live
added
the
blocked
|
To be abundantly clear: this is blocked on #1773 This proposal probably won't progress much until 1773 lands. |
As per 1773.
turt2live
removed
the
blocked
|
Team member @turt2live has proposed to merge this. The next step is review by the rest of the tagged people: No concerns currently listed. Once a majority of reviewers approve (and none object), 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 info about what commands tagged team members can give me. |
mscbot
added
proposed-final-comment-period
|
it still feels very strange to me that we do not distinguish old versions (eg v1) from future experimental versions. if nothing else i’d expect clients to show completely different messages: “this is on an old vulnerable room version and must be upgraded” versus “this room is on a new unstable version and may not work reliably”. |
|
It's impossible for the server to make that distinction now without opening up the room versions can of worms again, which I'm not going to be happy to do at this stage. The way the spec is worded for |
|
grr, fatfingered |
ara4n
added
proposed-final-comment-period
this feels incredibly anaemic and limited to me, and is going to result in awful client UX where clients have to say “uh, you’re on some kind of funny room version. do you want to switch it to the default?”, rather than “you are on a known vulnerable room version; upgrade now!” or “btw this room uses an experimental room version”. Surely it’s going to perpetrate the problem of the “vuln room warning!!” appearing everywhere - even when you upgrade to vN+1 as we have been doing over the last few weeks? However, given additional labels can be easily added in future, and how opinionated the inputs have ended up being here, i guess i’m okay for it to ship in this state albeit with the expectation that we’ll have to fix it down the line, and the argument will be clearer then. |
Maybe we can say that any value other than "stable" should be considered to be unstable, allowing us to add more values in the future. |
I'll put this in as it mirrors what the spec's intention is. I thought I put it in here though :( |
|
Registering M's concern: @mscbot concern stable vs unstable ux |
|
this doesn’t fix my point that i believe that for clients to provide good UX, they need to know whether to tell admins that their room version is outdated and vulnerable and needs upgrading - as opposed to being on a beta version which isn’t yet correct (and indeed is counterproductive) to tell people to upgrade from. it feels like in the excitement to fix the overlap between recommendedness and maturity, maturity got entirely thrown out, breaking this MSC. |
|
Discussed this OOB to try and figure out the requirements a bit more. Clients would end up treating unspecced versions as unsafe (intended) as one route for ensuring that the ecosystem isn't fragmented with weird room versions all over the place. The downside being that development of these room versions could be tiring (although in a development environment, one controls how their version is represented anyhow). @ara4n please correct me if I've missed anything or am wildly off point. |
|
mscbot looks to have gotten stuck, so in an attempt to fix it I'm asking for another FCP and mirroring the checkboxes - maybe it'll work. Original FCP here: #1804 (comment) @mscbot fcp merge |
|
Team member @turt2live has proposed to merge this. The next step is review by the rest of the tagged people: No concerns currently listed. Once a majority of reviewers approve (and none object), 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 info about what commands tagged team members can give me. |
mscbot
added
the
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
mscbot
removed
the
proposed-final-comment-period
|
The final comment period, with a disposition to merge, as per the review above, is now complete. |
mscbot
removed
the
final-comment-period
Original proposals: * #1753 * #1804 Implementation proof: * matrix-org/synapse#4472 * matrix-org/matrix-js-sdk#830 There is one change to MSC1753 which is included in this commit. MSC1804 remains unchanged. In the original proposal, the change password capability being present was an indication that password changes were possible. It was found that this doesn't really communicate the state very well to clients in that lack of a capability (or a 404, etc) would mean that users would erroneously not be able to change their passwords. A simple boolean flag was added to assist clients in detecting this capability.
turt2live
added
spec-pr-in-review
|
Merged via #1829 🎉 |
Rendered
Note: this is effectively blocked on room versions landing in the spec to ease understanding. Please read the spec PR for room versions before this one, otherwise things might not make any sense.