Also check the unstable feature for MSC3916

[S7evinK]

This allows clients to use authed media with server implementations not announcing support for v1.11 but still having support for MSC3916. (for example Dendrite)

Checklist

• Tests written for new code (and old code if feasible).
• New or updated public/exported symbols have accurate TSDoc documentation.
• Linter and other CI checks pass.
• Sign-off given on the changes (see CONTRIBUTING.md).

[richvdh]

Is this written down anywhere? I feel like it should be in the MSC if we're going to support it

[S7evinK]

Only asked internally if this was fine, and @dbkr seemed fine with it? Also the matrix-dart-sdk is doing the same, which I (personally) think is a sensible thing to do.

[dbkr]

Ah, I just gave some context on why it was the way that it was. @richvdh is right though, this prefix isn't mentioned anywhere in the MSC... where is it from? Also the PR title says 'unstable' but the prefix says 'stable'... and also it's a prefix so, by definition, not stable... I'm very confused :)

[S7evinK]

Then again, why has it ever existed in the unstable section, if it wasn't mentioned in the MSC?

[neilalexander]

this prefix isn't mentioned anywhere in the MSC... where is it from?

I feel like it should be in the MSC if we're going to support it

Ideally every single MSC that adds a feature should have an org.matrix.stable.mscXXXX or org.matrix.mscXXXX-style capability identifier automatically assigned as the rule, not the exception. Otherwise it's nigh-on impossible for homeserver implementations to declare partial support for a spec version or for specific features.

Once upon a time, graceful degradation was supposed to be a pillar of the Matrix protocol design, but that's simply and demonstrably not possible without finer-grained capability negotiation like this. It should be possible for a server to state either support for MSC3916 or support for spec version 1.11 (in which case, the feature is implicit and the capability identifier does not need to be called out separately) and have clients do the right thing.

[deepbluev7]

I think supporting this flag is pretty important from an ecosystem perspective, because otherwise homeservers, that are "behind" in spec support can't enable authenticated media.

The stable/unstable identifier isn't in the spec, but it is a convention that came up in a discussion between FluffyChat/dart-matrix-sdk and Conduit, that other homeserver developers seem to also have been involved in. The MSC didn't have any stable flags, but imo to support a quick rollout of authenticated media, probably should have one.

Without this flag, other homeservers have the choice of lying about their version support and claim support for 1.11 with only partial support for it or not supporting authenticated media in Element clients at all. I think the better option is to standardize clients on also supporting the unstable flag for a while until server support is somewhat caught up to recent Matrix versions.

If you can agree with that, I guess we could move the conversation then to how we move this PR forward. Imo we have 3 options:

• We pick this "stable" identifier as an informal standard
• We do another MSC just for the stable flag
• We do a PR to the merged MSC and add the stable flag after the fact without an MSC

The first option can imo also be done in parallel with the other options. But I think it would be a very good thing for the ecosystem, if Element supported this flag and imo the maintenance burden of it seems to not be that high.

I hope it is okay for me to throw my opinion on this in here, if I am disrupting the review process, feel free to mark this comment as offtopic. Thank you!

EDIT: I opened matrix-org/matrix-spec-proposals#4180, which may help this discussion.

[richvdh]

Ideally every single MSC that adds a feature should have an org.matrix.stable.mscXXXX or org.matrix.mscXXXX-style capability identifier automatically assigned as the rule, not the exception. Otherwise it's nigh-on impossible for homeserver implementations to declare partial support for a spec version or for specific features.

This isn't really the place for this discussion, but hard disagree. That sort of spec fragmentation is exactly what happened to XMPP, and it didn't go well.

I'm prepared to make an exception in this case, but I see this as a dangerous pattern.

[richvdh]

Then again, why has it ever existed in the unstable section, if it wasn't mentioned in the MSC?

well, great question. I guess we hallucinated this bit of the MSC. Wrongs of the past don't excuse more wrongs though.

[S7evinK]

https://github.com/element-hq/synapse/blob/f1a1c7fc537514ce5919d04b492ad3b2dba74646/synapse/rest/client/versions.py#L177

"past". Where did it come from, where did it go?

[richvdh]

https://github.com/element-hq/synapse/blob/f1a1c7fc537514ce5919d04b492ad3b2dba74646/synapse/rest/client/versions.py#L177

"past". Where did it come from, where did it go?

Not really following your point here.

Anyway now that this is in the MSC doc thanks to Nico, I'm happier about it landing here. It would be good to expand the PR title so that it reads better in the changelog.

[t3chguy]

This same change would likely need mirroring to element-desktop which does not run the serviceworker

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}