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

Deleting an edited message inconsistent between Ement.el and Element #228

Closed
phil-s opened this issue Sep 29, 2023 · 5 comments
Closed

Deleting an edited message inconsistent between Ement.el and Element #228

phil-s opened this issue Sep 29, 2023 · 5 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@phil-s
Copy link

phil-s commented Sep 29, 2023

Again, like #226 and #227. Apologies for the brevity on this one; hopefully you can reproduce easily.

As before, have both clients running together...

Editing and deleting a message using Element

Using Element:

  1. write a new message
  2. edit the message
  3. delete the message

Observe that over in Ement.el, the message was not deleted -- not unless you additionally delete it using Ement.el.

Editing and deleting a message using Ement.el

Using Ement.el:

  1. write a new message
  2. edit the message
  3. delete the message

Observe that over in Element you now have two different events:

  1. The original (un-edited) message is now visible again. (Presumably because the edit was deleted.)
  2. Below it is a "Message deleted" event.
@alphapapa alphapapa self-assigned this Sep 29, 2023
@alphapapa alphapapa added the bug Something isn't working label Sep 29, 2023
@alphapapa alphapapa added this to the 0.13 milestone Sep 29, 2023
@alphapapa
Copy link
Owner

Thanks again.

@alphapapa
Copy link
Owner

Well, I guess we'll have to look more closely at the spec. It's unclear to me what's happening here. For example, are we allowed to redact an edit? If so, does just the edit go away, and the original message return to its previous state? Or are we supposed to redact the original event ID, even if it was subsequently edited?

Much of the code in Ement was written to pass around event structs and get the ID and other fields from them as required; fixing this may require changing some more places to accept events or IDs as arguments, or having some functions "magically" use the ID of the original event, or having some interactive forms do that ID-resolving before calling the functions that do the work. It should be relatively straightforward, but the best solution is not yet clear.

@phil-s
Copy link
Author

phil-s commented Sep 30, 2023

https://spec.matrix.org/v1.8/client-server-api/#redactions-of-edited-events says:

Redactions of edited events

When an event using a rel_type of m.replace is redacted, it removes that edit revision. This has little effect if there were subsequent edits. However, if it was the most recent edit, the event is in effect reverted to its content before the redacted edit.

Redacting the original message in effect removes the message, including all subsequent edits, from the visible timeline. In this situation, homeservers will return an empty content for the original event as with any other redacted event, and as above the replacement events will not be included in the aggregation bundled with the original event. Note that the subsequent edits are not actually redacted themselves: they simply serve no purpose within the visible timeline.

alphapapa added a commit that referenced this issue Oct 3, 2023
Use in (ement-room-edit-message).

See #226, #227, #228.

Reported-by: Phil Sainty <phil@catalyst.net.nz>
alphapapa added a commit that referenced this issue Oct 3, 2023
See #226, #227, #228.

Reported-by: Phil Sainty <phil@catalyst.net.nz>
alphapapa added a commit that referenced this issue Oct 3, 2023
Seems like the correct thing to do.  See
<https://spec.matrix.org/v1.8/client-server-api/#redactions-of-edited-events>.

Fixes #228.

Reported-by: Phil Sainty <phil@catalyst.net.nz>
alphapapa added a commit that referenced this issue Oct 3, 2023
See #226, #227, #228.

Reported-by: Phil Sainty <phil@catalyst.net.nz>
@alphapapa
Copy link
Owner

I think that does it. I tested as you described and it all seems to work correctly in both clients now. Please let me know what you find out. Thanks for reporting these issues.

@phil-s
Copy link
Author

phil-s commented Oct 3, 2023

Excellent. I've done some basic testing and it looks good to me now -- things deleted in Ement.el show as deleted in Element as well, and vice versa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants