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

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

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

Comments

@phil-s
Copy link

phil-s commented Sep 28, 2023

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.

@phil-s phil-s added the bug Something isn't working label Sep 28, 2023
@purplg
Copy link

purplg commented Sep 28, 2023

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

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

Thanks to all for reporting.

@phil-s
Copy link
Author

phil-s commented Sep 29, 2023

Confirming the fix works for me. Thanks!

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

Reported-by: Phil Sainty <phil@catalyst.net.nz>
@alphapapa alphapapa added this to the 0.13 milestone Oct 3, 2023
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

3 participants