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

Replying to an edited message causes "This event could not be displayed" in Element #227

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

Comments

@phil-s
Copy link

phil-s commented Sep 29, 2023

Seems similar to #226.

OS/platform

Ubuntu GNU/Linux 22.04

Emacs version and provenance

Emacs 29.1 compiled from source.

Emacs command

emacs

Emacs frame type

GUI

Ement package version and provenance

Tested with current HEAD:

commit 99ee11a67fe2142a6fe401063141a449ba1acfe2
Author: Adam Porter <adam@alphapapa.net>
Date:   Thu Sep 28 13:23:52 2023 -0500

  Fix: (ement-room-edit-message) Already-edited events

Actions taken

  • Connect to session in both Ement and Element and observe side-by-side. Now, using Ement.el...
  • Write message, e.g. test. Both clients show test.
  • Edit message to test2. Both clients show test2 [edited]
  • Now write a reply to the edited message from each client in turn.

Observed results

Ement.el displays something like this for both replies:

In reply to @phil:<host>
> test2

reply 

While Element shows something like this for the reply sent by Ement.el:

In reply to @phil:<host>
> This event could not be displayed

reply 

Expected results

Element displays test2 as the quote.

Backtrace

No response

Etc.

This looks like much the same issue as #226. Looking at the events...

  • test has ID $ox7euKl9PzczuoJbGwHpTcn2AOV3GatwlcdWwCu2rOA
  • test2 has ID $hSiAg7wawUv6d-LbQgzEcHmoLXxCTu167kjMEUoOR4g
  • The reply to test2 from Ement.el has:
(m.relates_to
  (m.in_reply_to
    (event_id . "$hSiAg7wawUv6d-LbQgzEcHmoLXxCTu167kjMEUoOR4g")))`
  • The reply to test2 from Element has
(m.relates_to
   (m.in_reply_to
    (event_id . "$ox7euKl9PzczuoJbGwHpTcn2AOV3GatwlcdWwCu2rOA")))
@alphapapa
Copy link
Owner

@phil-s Thanks. Seems that we follow Postel's Law a bit more than Element does.

BTW, I didn't intend that experienced users would have to fill out the whole bug template when reporting issues that are plainly, actually bugs that simply need to be fixed, so feel free to skip the template and file a free-form report if you like.

@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 added a commit that referenced this issue Oct 2, 2023
Use in (ement-room-edit-message).

Fixes #227.

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

@phil-s Please see fa55808, which seems to fix it for me (having tested viewing the edit in Element as well). Please let me know if it does for you. Thanks.

@phil-s
Copy link
Author

phil-s commented Oct 3, 2023

I'll try again this evening when I have more time in case I messed up the update (testing with e3f5e0e origin/wip/original-id-of), but my initial test didn't fix the problem.

  • add message using ement.el
  • edit message using ement.el
  • reply to edited message using ement.el
  • check result in Element

Element still shows me This event could not be displayed as the quoted text.

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
Copy link
Owner

@phil-s Ok, I pushed another commit to this branch: https://github.com/alphapapa/ement.el/compare/wip/original-id-of I tested it according to your instructions, and it seems to work correctly now.

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
See #226, #227, #228.

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

phil-s commented Oct 3, 2023

Yep, that's done the trick! Cheers for getting a new release out with all those fixes.

I'm only seeing one other related point of difference between the two clients, which is that Element seems to dynamically update the quoted text in a reply when the message being quoted is edited (or deleted), whereas in Ement.el the quote remains unmodified. I'm not fussed about this myself -- it doesn't seem very important (certainly not the same class of inconsistency which these recent issues were resolving), and it's not clear to me that Element's behaviour is better. I'll leave it to you to log an issue for that if you think it's worth changing.

@alphapapa
Copy link
Owner

That issue should be handled by #150, which will fetch the content of referenced events rather than using the embedded, quoted content. However, as you said, I'm not convinced it's necessarily better, so we might have an option to disable that. Thanks.

@phil-s
Copy link
Author

phil-s commented Oct 7, 2023

As a footnote, I was just looking in the spec again trying to verify that replies need to target the original message ID, and I simply don't see that stated anywhere, so my impression is that this issue was on account of an Element bug (or if not that it's an accidental omission from the spec).

I believe the revised behaviour is perfectly valid though, and still think that it's the right thing for Ement.el to do for the sake of Element compatibility.

@alphapapa
Copy link
Owner

Yeah, a bit ambiguous, but this is probably best. Thanks.

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