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

Specify a means for clients to "edit" previous messages (SPEC-410) #184

Closed
matrixbot opened this issue Jun 9, 2016 · 24 comments
Closed
Labels
A-Client-Server Issues affecting the CS API feature Suggestion for a significant extension which needs considerable consideration

Comments

@matrixbot
Copy link
Member

Submitted by @​matthew:matrix.org

(Imported from https://matrix.org/jira/browse/SPEC-410)

@matrixbot
Copy link
Member Author

Jira watchers: @leonerd @richvdh

@matrixbot
Copy link
Member Author

matrixbot commented Jun 9, 2016

Links exported from Jira:

relates to SPEC-9
vector-web matrix-org/matrix-spec-proposals#1862

@matrixbot
Copy link
Member Author

As with redactions, this will presumably take the form of an event which refers to the original event.

-- @richvdh

@matrixbot
Copy link
Member Author

Since Gitter has the ability to perform message updates (it lets users edit their most recent message) the Matrix-Gitter bridge has to handle this. Currently it does it by performing a diff between the old and new content, stripping a common prefix and suffix and then presenting the resulting change on the Matrix side in a (somewhat) helpful way. I've made mention of this spec bug in the code there, so if a better means arrives in Matrix to represent this it can be updated.

See
matrix-org/matrix-appservice-gitter@0760134

-- @leonerd

@matrixbot
Copy link
Member Author

Pretty, pretty please with sugar on top add this soon.

-- Andrew

@matrixbot matrixbot changed the title Specify a means for clients to "edit" previous messages Specify a means for clients to "edit" previous messages (SPEC-410) Oct 31, 2016
@matrixbot matrixbot added the feature Suggestion for a significant extension which needs considerable consideration label Nov 7, 2016
@enyo
Copy link

enyo commented Jan 25, 2017

Is there a branch or a PR regarding this issue where we could follow implementation of this issue?
This is the last feature that I think is really severely missing from matrix.

@ara4n
Copy link
Member

ara4n commented Feb 6, 2017

there's a contribution from @pik at matrix-org/matrix-react-sdk#588 but we haven't yet got to properly review or merge it, as the core team is drowning in E2E bughunting etc. Agreed that it's a hugely important feature.

@MurzNN
Copy link
Contributor

MurzNN commented Aug 16, 2017

Maybe in near future you will have time to review this contribution? Edit message feature will be very useful for people, especially for blog-like rooms via Matrix Live or Journal

@NotAFile
Copy link

NotAFile commented Sep 18, 2017

Some discussion with @richvdh revealed the following:

  • You can not naively edit matrix messages, since messages are identified by their HMAC. Changing the message thus invalidates it. This means that the only way to implement editing is to send a second event.

  • You can not naively send an edit event, as it might be missed by the client and homeserver. This would happen e.g. when permalinking to an old message that was edited later.

There appear to be three solutions:

  • message aggregation: I have no idea what this means or entails, but it was mentioned. This is apparently a difficult solution that also enables reactions and slack-style threading.

  • Limiting the amount of time/messages between a message and an edit and forcing the homeserver to backfetch those messages. While hardly elegant, this is a simple solution. Since most messages are edited immediately after sending, it should work quite well.

  • Invalidating the original event with a link to the current one. This seems like the most promising solution to me. The original event is pseudo-redacted and a new attribute, e.g. "superseded_by" or "edited" is added with a link to the replacement event. This means the HMAC would no longer match, but this is fine because the new event has a valid HMAC.

If no objections are made, I hope to make a proposal for implementing the last solution soon.

Other issues:

  • privacy vs transparency: while a message edit history is nice, users might expect edits to destroy the previous message, which is unfortunate if it contains accidentally shared information.

  • several edits: The easiest way here would be the server redacting the previous edit event when another edit is made

  • Bridged protocols without support for message editing. When bridging to e.g. IRC, there is no good way to deal with edits. Sending a new message for a one letter typo fix is bound to annoy other users, while not relaying an edit might lead to miscommunication

@romainf-unity
Copy link

romainf-unity commented Nov 20, 2017

Hope I'm not intruding here. I've read your very interesting post @NotAFile and that made me thinking.

Disclaimer: I'm merely proposing a solution from my perspective, hoping it's also what most of the users would like to have as well. Although I'm a programmer myself, I reckon I don't know anything about decentralized messaging apps 😄

From a user perspective, I think the user should have all the rights over his messages. And by that, I think this is what Matrix should be aiming to. Of course, there is always the problem of a third party server that don't follow the protocol and, for instance, keeps all the versions of the messages, even if the user asked to delete it... but we can't do anything against that, can we?

So, with that in mind, I think a solution could be to see the history of the messages. This is helpful for both the author of the message, but also the other users that can see what has been edited (that would actually be a feature Slack doesn't have).

By being able to see the history of his message, the user will understand that editing the message simply adds a new "revision" to the message, but if he wants to delete the message entirely, he'll have to delete his message (with a confirmation window, showing that this is "more permanent" than a simple edit).

@MurzNN
Copy link
Contributor

MurzNN commented Nov 20, 2017

I support proposal for editing message via adds new revision (like comments in Facebook)- this will be best way. Maybe this can be implemented via Threaded Messaging feature, with special mark that this is thread for message revision, and display only last revision for this thread type.

@AndrewJDR
Copy link

AndrewJDR commented Apr 6, 2018

Would love to see this in soon. I did want to point out one potential complication with this that I don't think has been mentioned:

For messages that were composed using markdown syntax in riot, the markdown text gets converted to html and stored in the formatted_body attribute of the message event, and I believe this happens on the client side before the event is sent to the homeserver.

When starting an editing on a previous message, you'd presumably want to edit it the same way you originally composed it.. for e.g., using the markdown syntax you originally used. The problem is, the way things work now, that original syntax is effectively gone by the time you go to edit the message -- as just mentioned, it was converted to html before being sent to the server.

Something would have to be done about this. That might mean storing the original text (i.e. markdown) used to compose every message in the events that are sent to the home server. To go further with it, perhaps the original syntax used to compose all messages should be the only thing send to the server, and it should be converted to html on the receiving end. Or something else I haven't thought of. Perhaps a matrix dev can chime in with some ideas/comments.

In any case, It does seem like a significant complication to me, unless I'm missing something obvious.

@NotAFile
Copy link

NotAFile commented Apr 6, 2018

@AndrewJDR The messages include both a formatted_body and a body attribute. The body contains the raw text, including markdown formatting. So that's not an issue. This is presumably done so e.g. Bots don't need to have to parse html

@ghost
Copy link

ghost commented Apr 6, 2018

The body contains the raw text, including markdown formatting

Not really. The body should contain a plain-text representation of the formatted_body, which may be styled as markdown source code. Or not. Depends on the client that sends it.

@maxidorius
Copy link
Contributor

maxidorius commented Apr 6, 2018

@Matrixcoffee not quite. body should contain whatever is sent, and the client is free to parse and apply formatting in a dedicated key formatted_body.
See the spec which doesn't mandate plain text and only claims that body is The body of the message.

Therefore formatted_body is a post-computation key of body, not the other way around.
You'll also notice that formatted_body is actually not part of the spec and is a Riot thing only.

@turt2live
Copy link
Member

formatted_body is not a Riot thing only. matrix-org/matrix-spec-proposals#917

@maxidorius
Copy link
Contributor

Well it's not part of the spec, and Riot was the first client that started using it and pretty much decide what is valid for the org.matrix.custom.html format type... so yes, it is a Riot thing only.

@turt2live
Copy link
Member

matrix-console (which preceded Riot) also used it: https://github.com/matrix-org/matrix-angular-sdk/blob/84b0b64d3a6b3ecb602783123481cdca65a78e3e/syweb/webclient/lib/matrix.js#L171-L179

In any case, the ongoing recommendation (which obviously should be added to the spec, like everything else) is that the body be a representation of the event. It makes sense that it could be markdown because markdown is meant to be human-readable. I haven't looked into it too much, but I do believe that most clients send nearly exactly what the user entered in the body anyways.

@ghost
Copy link

ghost commented Apr 6, 2018

@turt2live: There are tiny little niggles like stripping backslashes. After all, if I send a shrug emoticon, I'd expect the plain body to contain ~\_(ツ)_/~ not ~\\\_(ツ)\_/~ so it looks right in dumb text-only clients, including IRC bridge ASes.

@KitsuneRal
Copy link
Member

KitsuneRal commented Apr 6, 2018

Isn't the proposal about event structure supposed to officially fix an event representation that could be used for editing?

@NotAFile
Copy link

matrix-org/matrix-spec-proposals#1695 should probably be linked here

@richvdh
Copy link
Member

richvdh commented Sep 6, 2019

this will be fixed by matrix-org/matrix-spec-proposals#1849 once it is done.

@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 1, 2022
@richvdh
Copy link
Member

richvdh commented Apr 19, 2022

or, now that MSC1849 has been superceded, matrix-org/matrix-spec-proposals#2676.

@richvdh
Copy link
Member

richvdh commented Nov 14, 2022

fixed by #1211

@richvdh richvdh closed this as completed Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Client-Server Issues affecting the CS API feature Suggestion for a significant extension which needs considerable consideration
Projects
None yet
Development

No branches or pull requests