After repeated edits in Ement, a message becomes hidden/missing in Element

phil-s commented 9 months ago

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

Issue verified with ement-0.12 from GNU ELPA via package-install

Actions taken

Connect to session in both Ement and Element and observe side-by-side.
Write message, e.g. test. Both clients show test.
Edit message to test2. Both clients show test2 [edited]
Edit message again to test3. Ement shows test3 [edited]. Element no longer shows the message text at all, although my username is still present at that position.

Observed results

Element fails to render an edit of an edit, while Ement does display it. This suggests that I may have been inadvertently hiding my messages from others without realising it.

Expected results

Element displays the twice-edited message.

Backtrace

No response

Etc.

I can edit messages repeatedly in Element without problem.

Looking at event data, I think the difference is this:

When I create a message with ID foo and repeatedly edit it in Element, the event for each and every edit says:

(m.relates_to
   (event_id . "foo")
   (rel_type . "m.replace"))

Conversely when I create a message with ID foo and repeatedly edit it in Ement, each event relates_to the distinct ID of the previous edit event in the series; so only the first edit looks like the example above, and each subsequent edit event (i.e. all of the ones which Element doesn't display) have a different event_id value in that section.

purplg commented 9 months ago

We discovered in the Ement.el chatroom that edit events should reference the original message, not the edited message.

Per the spec:

The original event must not, itself, have a rel_type of m.replace (i.e. you cannot edit an edit — though you can send multiple edits for a single original event).

https://spec.matrix.org/v1.8/client-server-api/#validity-of-replacement-events

alphapapa commented 9 months ago

(defun ement-room-edit-message (event room session body)
  "Edit EVENT in ROOM on SESSION to have new BODY.
The message must be one sent by the local user."
  (interactive (ement-room-with-highlighted-event-at (point)
                 (cl-assert ement-session) (cl-assert ement-room)
                 (pcase-let* ((event (ewoc-data (ewoc-locate ement-ewoc)))
                              ((cl-struct ement-session user events) ement-session)
                              ((cl-struct ement-event sender
                                          (content (map body ('m.relates_to
                                                              (map ('event_id replaced-event-id)
                                                                   ('rel_type relation-type))))))
                               event)
                              (ement-room-editing-event event))
                   (unless (equal (ement-user-id sender) (ement-user-id user))
                     (user-error "You may only edit your own messages"))
                   (pcase relation-type
                     ("m.replace"
                      ;; Editing an already-edited event: get the original event.
                      (setf event (or (gethash replaced-event-id events)
                                      (error "Can't find original event <%s> to edit" replaced-event-id)))))
                   ;; Remove any leading asterisk from the plain-text body.
                   (setf body (replace-regexp-in-string (rx bos "*" (1+ space)) "" body t t))
                   (ement-room-with-typing
                     (let* ((prompt (format "Edit message (%s): "
                                            (ement-room-display-name ement-room)))
                            (body (ement-room-read-string prompt body 'ement-room-message-history
                                                          nil 'inherit-input-method)))
                       (when (string-empty-p body)
                         (user-error "To delete a message, use command `ement-room-delete-message'"))
                       (when (yes-or-no-p (format "Edit message to: %S? " body))
                         (list event ement-room ement-session body)))))))
  (let* ((endpoint (format "rooms/%s/send/%s/%s" (url-hexify-string (ement-room-id room))
                           "m.room.message" (ement--update-transaction-id session)))
         (new-content (ement-alist "body" body
                                   "msgtype" "m.text"))
         (_ (when ement-room-send-message-filter
              (setf new-content (funcall ement-room-send-message-filter new-content room))))
         (content (ement-alist "msgtype" "m.text"
                               "body" body
                               "m.new_content" new-content
                               "m.relates_to" (ement-alist "rel_type" "m.replace"
                                                           "event_id" (ement-event-id event)))))
    ;; Prepend the asterisk after the filter may have modified the content.  Note that the
    ;; "m.new_content" body does not get the leading asterisk, only the "content" body,
    ;; which is intended as a fallback.
    (setf body (concat "* " body))
    (ement-api session endpoint :method 'put :data (json-encode content)
      :then (apply-partially #'ement-room-send-event-callback :room room :session session
                             :content content :data))))

This seems to fix it. Will push later unless someone says that it doesn't work...

alphapapa commented 9 months ago

Thanks to all for reporting.

phil-s commented 9 months ago

Confirming the fix works for me. Thanks!

alphapapa / ement.el