Custom emotes

[nmscode]

Checklist

• Tests written for new code (and old code if feasible)
• Linter and other CI checks pass
• Sign-off given on the changes (see CONTRIBUTING.md)

Spec proposal
Also has a compatibility mode with MSC2545

Can be accessed on https://pr9240--matrix-react-sdk.netlify.app/
Type: enhancement
More details in this comment:
element-hq/element-meta#339 (comment)

Signed off by: nmscode [email protected]

---

Here's what your changelog entry will look like:

✨ Features

• Custom emotes (#9240). Contributed by @nmscode.

[robintown]

Thank you for contributing! You appear to have somehow commited the entire contents of the react-sdk as a brand new repository - we will be unable to review this until that is fixed. We also require all changes to be signed off, per the checklist.

It's also important to note that there are other PRs adding custom emoji functionality to the app, such as #8087, that this would likely conflict with. Additionally, any changes to the Matrix spec, such as a hypothetical m.room.emotes event type as is used here, need to go through the spec proposal process first. There are already several spec proposals for custom emoji out there, such as MSC2545, though the approach to custom emoji as a whole has not had much attention from the spec core team yet, and is very much subject to change.

[nmscode]

I apologize for the issue with the repository. I have rebased it and I think that particular issue is fixed now.

[turt2live]

Thank you for the contribution, however this doesn't appear to be in a reviewable state at the moment: there's merge conflict chunks, incorrect indentation, linter errors, etc which all need to be addressed before we can take a closer look at this.

[Johennes]

Thank you for your contribution and thanks also for your patience to hear back from us. @daniellekirkwood and I discussed your pull request this week and concluded today.

TL/DR: We'd like to proceed with this pull request in general but we're gonna block it for now.

The way it is implemented, custom emoji are sent as short codes only with the rendering happening on the receiving side. This creates a rather large vector for abuse and we're not gonna accept it in this form.

Separately from that, the Spec Core Team expects to land custom emoji / sticker packs in Matrix v1.9 which is due in November. Given how close this is, we think that it's worth blocking this contribution to align on a proper solution on the spec level. I'd encourage you to engage with the spec process on the various MSCs around this feature to help influence the decision making.

We will use the meantime to apply some basic UX thinking around this feature. For instance, finding a way to not over-complicate the room settings and an integration with (canonical) spaces would be quite desirable.

Once the spec discussions have converged and assuming you're still interested, we'd be happy to work with you to get this pull request into a form that we'd land. In the meantime, I will convert this pull request into a draft.

[nmscode]

The way it is implemented, custom emoji are sent as short codes only with the rendering happening on the receiving side. This creates a rather large vector for abuse and we're not gonna accept it in this form.

This is complete nonsense for two reasons. One, there isn't any "large vector for abuse" with the client rendering approach. Two, there is a compatibility mode that does not send the emoji as shortcodes that I specifically added to alleviate any concerns about the approach. The compatibility mode could have been made the default if that was truly the concern.

Separately from that, the Spec Core Team expects to land custom emoji / sticker packs in Matrix v1.9 which is due in November. Given how close this is, we think that it's worth blocking this contribution to align on a proper solution on the spec level. I'd encourage you to engage with the spec process on the various MSCs around this feature to help influence the decision making.

November is close? Given all the delays adding another two months may seem relatively close, but the time this feature has already taken is ridiculous. I mainly wanted this PR in labs so users could try it out and have feedback for the spec review process so waiting until spec review is done does not make sense.

We will use the meantime to apply some basic UX thinking around this feature. For instance, finding a way to not over-complicate the room settings and an integration with (canonical) spaces would be quite desirable.

The UX is not an issue since it would have been in labs and honestly it is not that complicated as it is right now. Spaces have so many issues that should be fixed before custom emotes can be integrated, which is why I only did it for rooms.

Once the spec discussions have converged and assuming you're still interested, we'd be happy to work with you to get this pull request into a form that we'd land. In the meantime, I will convert this pull request into a draft.

Unfortunately I am no longer interested. I will be closing this since it is clear custom emotes are not happening any time soon, and I will not contribute further to Element in the future.

[williamkray]

another PR bites the dust.

the custom emotes topic has done more to push away potential element contributors than any other issue i'm aware of. for an organization who has publicly discussed having limited development resources and depending on donations as a funding model, i would imagine that maintaining the good-will of contributors who donate their time and resources for free would be a priority, but i guess not. that's a shame.

[Johennes]

[...] there isn't any "large vector for abuse" with the client rendering approach. Two, there is a compatibility mode that does not send the emoji as shortcodes that I specifically added to alleviate any concerns about the approach.

There rather clearly is one in my opinion and it's not mitigated by having a flag to turn it off. More importantly though the fact that we disagree on this reinforces our conclusion that this is more efficiently agreed upon on the spec level than here.

November is close? Given all the delays adding another two months may seem relatively close, but the time this feature has already taken is ridiculous.

I agree that it has taken us way too long to reach a decision on this pull request. That shouldn't force us to go head over heels now though. We need to balance our limited time and resources against having to maintain code once it has been merged.

I mainly wanted this PR in labs so users could try it out and have feedback for the spec review process so waiting until spec review is done does not make sense.

The MSC process doesn't require implementing MSCs in Element. You can use any client. We also don't have to wait for the formal end of the process or a spec release. It seems crucial in this case though to reach some alignment and convergence. The MSC you're referring to hasn't received any affirmative feedback from the community at all which doesn't create a lot of confidence that we're marching into the right direction here.

Unfortunately I am no longer interested. I will be closing this since it is clear custom emotes are not happening any time soon, and I will not contribute further to Element in the future.

I'm sorry you feel this way. We are interested in seeing custom emoji in Element at some point but currently lack the resources to put it onto our own roadmap. We would still be interested in working with community contributors towards this though. So please do get in touch should you change your mind.

[blotree]

God, what a tone-deaf response! You won't let others implement things and claim to not have the resources yourself. This has been an issue for literal years with many attempts by many people to get things moving in any direction only to be blocked at every turn. Not only are you being intransigent by not accepting proposed solutions to your concerns or even having the discussion about things but the mere fact that it takes weeks between responses from developers is utterly bonkers.

Further, you simply won't get any community feedback if you don't let the community actually test things. That can only be done by putting the feature in a visible place, not hidden in instances referred to in difficult-to-find PRs. In other words, lab features.

How do you expect people to contribute positively to the Matrix ecosystem if you won't give them a chance? Opaqueness and slow responses combined with the shutting down of any attempts at working through any concerns will just keep turning off prospective contributors.

[nmscode]

It seems crucial in this case though to reach some alignment and convergence. The matrix-org/matrix-spec-proposals#3892 you're referring to hasn't received any affirmative feedback from the community at all which doesn't create a lot of confidence that we're marching into the right direction here.

I would like to clarify here for anyone reading this that this PR was not just an implementation of MSC3892, but also had a compatibility mode that implemented/worked with MSC2545. MSC2545 has been used in several clients such as Cinny, Fluffy,etc. Several other, older PRs also implemented MSC2545 and met the same fate.

[Johennes]

I'm sorry and I understand that this has been a frustrating experience. I can't really speak to the past as I haven't even been on this team when the PR was created. This topic has made it to me mid-August by way of another person who was, unfortunately, made redundant. We've discussed and reached a decision on this and two other related PRs in two weeks. That strikes me as fairly reasonable but I understand your resentments about the past.

[williamkray]

@Johennes unfortunately you've been stuck onto a very unfortunate situation. the general tone you're seeing here can be better understood by following this conversation on the element-meta issue.

i'm sure it's nothing specifically about you, but rather general frustration with how element.io has handled this feature request for the last several years... or rather, not handled it, and now passed it to you without giving you the full context.

good luck, we're all very eager to see a resolution here!

[Johennes]

As I said above, we're happy to continue working with somebody on this but, sadly, we're not currently in a position to put it onto our own roadmap.

I stand by the assessment that the way it's done in this PR feels wrong. This has been pointed out by others above and we're not gonna pursue the MSC3892 route without buy-in on the MSC side – not least because we have a responsibility to keep our users safe.

MSC2545, while not formally accepted, seems less disputed and it would be much easier to consider a contribution that was based on that variant alone. So if somebody would like to fabricate one, feel free to get in touch with me.

[blotree]

MSC2545, while not formally accepted, seems less disputed and it would be much easier to consider a contribution that was based on that variant alone. So if somebody would like to fabricate one, feel free to get in touch with me.

Oh, you mean like the pull request #8087 or #8897? Both which were in limbo for various reasons and closed in favor of this PR? At this point it feels like attempting to start another PR risks potential contributors wasting their time for another year between slow reviews and responses. I'll ask point blank so it is clear: would an implementation of MSC2545 get pulled in? Yes or no, no maybes nor "we'll see" please.

[Johennes]

I'm sorry, I can relate to your frustration but you're really not being very helpful here. I think I've made it clear enough that I am here now to work with somebody who is genuinely interested in moving this forward. If you're not that person, I suggest you step aside for somebody who is rather than excavating things we cannot change anymore.

For what it's worth, as far as I can tell these other PRs have received neither pushback nor encouragement to be abandoned over this PR from our side. We should have actioned them and it's unfortunate that we didn't but, again, I can't go back in time.

I actually like the approach that #8087 is taking with leaving out the room settings part. This adds value while limiting the scope which makes it easier to get something agreed upon and merged. So if somebody wanted to pursue that, I'd be supportive of it.

[nmscode]

I'm sorry, I can relate to your frustration but you're really not being very helpful here. I think I've made it clear enough that I am here now to work with somebody who is genuinely interested in moving this forward. If you're not that person, I suggest you step aside for somebody who is rather than excavating things we cannot change anymore.

For what it's worth, as far as I can tell these other PRs have received neither pushback nor encouragement to be abandoned over this PR from our side. We should have actioned them and it's unfortunate that we didn't but, again, I can't go back in time.

I actually like the approach that #8087 is taking with leaving out the room settings part. This adds value while limiting the scope which makes it easier to get something agreed upon and merged. So if somebody wanted to pursue that, I'd be supportive of it.

I would like to see an answer to @blotree's question as well and I don't think people commenting about this should be told to "step aside". If the only thing holding this back is MSC3892 then I can remove that in favor of MSC2545 and we can move forward. I will, however, say that hinting that the MSC implementation "feels wrong" or is not "safe" for users seems vague and unhelpful.

So far you have mentioned MSC3892 being the issue and now it is that the PR messes with room settings? The changes to room settings are behind a lab flag and it is just adding a tab to let people upload emotes. #8087 does not allow uploading emotes from element. That is hardly a solution.

I will add that I talked to the author of #8087 who now refuses to contribute to Element as a rule now due to frustration with the Element team.

All of that being said, I will rephrase @blotree's question a little bit: Is there any combination of changes to the PR that will let this land in labs or do we have to wait until November's spec updates at the earliest for this?

[Johennes]

I will, however, say that hinting that the MSC implementation "feels wrong" or is not "safe" for users seems vague and unhelpful.

I have explained what about it feels unsafe in my initial response and others have commented on it here before.

So far you have mentioned MSC3892 being the issue and now it is that the PR messes with room settings? The changes to room settings are behind a lab flag and it is just adding a tab to let people upload emotes. #8087 does not allow uploading emotes from element. That is hardly a solution.

I have hinted that the UX in this PR is probably not what we'd like it to be in my first response. We're currently working on simplifying the room settings rather than extend them. Smaller PRs that do something useful with less scope are generally a good way to make progress.

Is there any combination of changes to the PR that will let this land in labs or do we have to wait until November's spec updates at the earliest for this?

As I said above, we're not gonna pursue the approach this PR currently takes without alignment on the MSC level. Thr SCT is planning to look at the custom emoji MSCs until November but you're welcome to engage with the process before then.

Otherwise, doing MSC2545-style sending looks like a good first step to me. Assuming the code and UX are up to standards, I'm positive that we could get that landed behind labs.

[nmscode]

I have explained what about it feels unsafe in my initial response and others have commented on it here before.

I went back and re-read your post. You did not really explain that. Nonetheless, it is clear you will not let MSC3892 through so I will not argue that it should stay.

I have hinted that the UX in this PR is probably not what we'd like it to be in my first response. We're currently working on simplifying the room settings rather than extend them. Smaller PRs that do something useful with less scope are generally a good way to make progress.

So you want people to be able to send custom emotes with no way of uploading or editing them in settings??? I understand simplifying but what you are saying does not make sense. There's a difference between simplifying and removing functionality. What is so wrong with the UX currently that we need to drop the entire functionality of uploading and editing the emotes? I think it's a perfectly acceptable start that can be modified later since this is in labs.

Otherwise, doing MSC2545-style sending looks like a good first step to me.

This is already in the PR. If MSC3892 stuff is removed it will be the default.

[Johennes]

I went back and re-read your post. You did not really explain that. Nonetheless, it is clear you will not let MSC3892 through so I will not argue that it should stay.

Ok, apologies if that wasn't clear enough. I think the place to argue about whether or not MSC3892 is the right way are the MSCs. I'd still encourage you to do that if you think it is. Competing proposals are highly valuable for reaching conclusions on the spec level.

So you want people to be able to send custom emotes with no way of uploading or editing them in settings??? I understand simplifying but what you are saying does not make sense. There's a difference between simplifying and removing functionality. What is so wrong with the UX currently that we need to drop the entire functionality of uploading and editing the emotes? I think it's a perfectly acceptable start that can be modified later since this is in labs.

Sorry, that's not what I meant. The room settings are an area we're planning to make updates in. Any larger change to it, labs or not, will make that more complicated and will require a certain level of coordination with our product and design team. If sending and settings were in two separate PRs, that would make things easier as we could land the sending part without blocking on the settings. That's not a requirement though. I'm just saying that it'd probably allow us to make faster and incremental progress.

This is already in the PR. If MSC3892 stuff is removed it will be the default.

Alright, if you're willing to make that change, please get in touch on element-hq/element-meta#339.