alphapapa / ement.el

A Matrix client for GNU Emacs
GNU General Public License v3.0
488 stars 44 forks source link

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

Closed phil-s closed 12 months ago

phil-s commented 1 year ago

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

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...

alphapapa commented 1 year ago

@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 commented 1 year ago

@phil-s Please see https://github.com/alphapapa/ement.el/commit/fa55808b2291d983154980b5215ea8a63d2e3f40, 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 commented 12 months ago

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.

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

alphapapa commented 12 months ago

@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.

phil-s commented 12 months ago

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 commented 12 months ago

That issue should be handled by https://github.com/alphapapa/ement.el/pull/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 commented 12 months ago

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 commented 12 months ago

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