Skip to content

Commit

Permalink
edit message's text (#6550)
Browse files Browse the repository at this point in the history
> _greetings from the ice of the deutsche bahn 🚂🚃🚃🚃 always a pleasure to
see how well delta chat meanwhile performs in bad networks :)_

this PR adds an API to request other chat members to replace the message
text of an already sent message. scope is mainly to fix typos. this
feature is known from whatsapp, telegram, signal, and is
[requested](https://support.delta.chat/t/retract-edit-sent-messages/1918)
[since](https://support.delta.chat/t/edit-messages-in-delta-chat/899)
[years](deltachat/deltachat-android#198).

technically, a message with an
[`Obsoletes:`](https://datatracker.ietf.org/doc/html/rfc2076#section-3.6)
header is sent out.

```
From: alice@nine
To: bob@nine
Message-ID: 2000@nine
In-Reply-To: 1000@nine
Obsoletes: 1000@nine

Edited: this is the new text
```

the body is the new text, prefixed by the static text `Edited:` (which
is not a header). the latter is to make the message appear more nicely
in Non-Delta-MUA. save for the `In-Reply-To` header. the `Edited:`
prefix is removed by Delta Chat on receiving.

headers should be protected and moved to e2ee part as usual.

corrected message text is flagged, and UI should show this state, in
practise as "Edited" beside the date.

in case, the original message is not found, nothing happens and the
correction message is trashes (assuming the original was deleted).
question: is the `Obsoletes:` header a good choice? i _thought_ there is
some more specifica RFC, but i cannot find sth. in any case, it should
be an header that is not used otherwise by MUA, to make sure no wanted
messages disappear.

what is NOT done and out of scope:
- optimise if messages are not yet sent out. this is doable, but
introduces quite some cornercaes and may not be worth the effort
- replaces images or other attachments. this is also a bit cornercasy
and beyond "typo fixing", and better be handled by "delete for me and
others" (which may come soon, having the idea now, it seems easy :)
- get edit history in any way. not sure if this is worth the effort,
remember, as being a private messenger, we assume trust among chat
members. it is also questionable wrt privacy, seized devices etc.
- add text where nothing was before; again, scope is "typo fixing",
better avoid cornercases
- saved messages are not edited (this is anyway questionable)
- quoted texts, that are used for the case the original message is
deleted, are not updated
- edits are ignored when the original message is not there yet (out of
order, not yet downloaded)
- message status indicator does not show if edits are sent out or not -
similar to reactions, webxdc updates, sync messages. signal has the same
issue :) still, connectivity should show if there are messages pending

<img width="366" alt="Screenshot 2025-02-17 at 17 25 02"
src="https://github.com/user-attachments/assets/a4a53996-438b-47ef-9004-2c9062eea5d7"
/>

corresponding iOS branch (no PR yet):
deltachat/deltachat-ios@main...r10s/edit-messages

---------

Co-authored-by: l <link2xt@testrun.org>
  • Loading branch information
r10s and link2xt authored Feb 21, 2025
1 parent 85cd383 commit 85cbfde
Show file tree
Hide file tree
Showing 11 changed files with 254 additions and 5 deletions.
31 changes: 31 additions & 0 deletions deltachat-ffi/deltachat.h
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,23 @@ uint32_t dc_send_msg_sync (dc_context_t* context, uint32
uint32_t dc_send_text_msg (dc_context_t* context, uint32_t chat_id, const char* text_to_send);


/**
* Send chat members a request to edit the given message's text.
*
* Only outgoing messages sent by self can be edited.
* Edited messages should be flagged as such in the UI, see dc_msg_is_edited().
* UI is informed about changes using the event #DC_EVENT_MSGS_CHANGED.
* If the text is not changed, no event and no edit request message are sent.
*
* @memberof dc_context_t
* @param context The context object as returned from dc_context_new().
* @param msg_id The message ID of the message to edit.
* @param new_text The new text.
* This must not be NULL nor empty.
*/
void dc_send_edit_request (dc_context_t* context, uint32_t msg_id, const char* new_text);


/**
* Send invitation to a videochat.
*
Expand Down Expand Up @@ -4432,6 +4449,20 @@ int dc_msg_is_sent (const dc_msg_t* msg);
int dc_msg_is_forwarded (const dc_msg_t* msg);


/**
* Check if the message was edited.
*
* Edited messages should be marked by the UI as such,
* e.g. by the text "Edited" beside the time.
* To edit messages, use dc_send_edit_request().
*
* @memberof dc_msg_t
* @param msg The message object.
* @return 1=message is edited, 0=message not edited.
*/
int dc_msg_is_edited (const dc_msg_t* msg);


/**
* Check if the message is an informational message, created by the
* device or by another users. Such messages are not "typed" by the user but
Expand Down
27 changes: 27 additions & 0 deletions deltachat-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,23 @@ pub unsafe extern "C" fn dc_send_text_msg(
})
}

#[no_mangle]
pub unsafe extern "C" fn dc_send_edit_request(
context: *mut dc_context_t,
msg_id: u32,
new_text: *const libc::c_char,
) {
if context.is_null() || new_text.is_null() {
eprintln!("ignoring careless call to dc_send_edit_request()");
return;
}
let ctx = &*context;
let new_text = to_string_lossy(new_text);

block_on(chat::send_edit_request(ctx, MsgId::new(msg_id), new_text))
.unwrap_or_log_default(ctx, "Failed to send text edit")
}

#[no_mangle]
pub unsafe extern "C" fn dc_send_videochat_invitation(
context: *mut dc_context_t,
Expand Down Expand Up @@ -3683,6 +3700,16 @@ pub unsafe extern "C" fn dc_msg_is_forwarded(msg: *mut dc_msg_t) -> libc::c_int
ffi_msg.message.is_forwarded().into()
}

#[no_mangle]
pub unsafe extern "C" fn dc_msg_is_edited(msg: *mut dc_msg_t) -> libc::c_int {
if msg.is_null() {
eprintln!("ignoring careless call to dc_msg_is_edited()");
return 0;
}
let ffi_msg = &*msg;
ffi_msg.message.is_edited().into()
}

#[no_mangle]
pub unsafe extern "C" fn dc_msg_is_info(msg: *mut dc_msg_t) -> libc::c_int {
if msg.is_null() {
Expand Down
61 changes: 60 additions & 1 deletion src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::color::str_to_color;
use crate::config::Config;
use crate::constants::{
self, Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK,
DC_CHAT_ID_LAST_SPECIAL, DC_CHAT_ID_TRASH, DC_RESEND_USER_AVATAR_DAYS,
DC_CHAT_ID_LAST_SPECIAL, DC_CHAT_ID_TRASH, DC_RESEND_USER_AVATAR_DAYS, EDITED_PREFIX,
TIMESTAMP_SENT_TOLERANCE,
};
use crate::contact::{self, Contact, ContactId, Origin};
Expand Down Expand Up @@ -3142,6 +3142,65 @@ pub async fn send_text_msg(
send_msg(context, chat_id, &mut msg).await
}

/// Sends chat members a request to edit the given message's text.
pub async fn send_edit_request(context: &Context, msg_id: MsgId, new_text: String) -> Result<()> {
let mut original_msg = Message::load_from_db(context, msg_id).await?;
ensure!(
original_msg.from_id == ContactId::SELF,
"Can edit only own messages"
);
ensure!(!original_msg.is_info(), "Cannot edit info messages");
ensure!(
original_msg.viewtype != Viewtype::VideochatInvitation,
"Cannot edit videochat invitations"
);
ensure!(
!original_msg.text.is_empty(), // avoid complexity in UI element changes. focus is typos and rewordings
"Cannot add text"
);
ensure!(!new_text.trim().is_empty(), "Edited text cannot be empty");
if original_msg.text == new_text {
info!(context, "Text unchanged.");
return Ok(());
}

save_text_edit_to_db(context, &mut original_msg, &new_text).await?;

let mut edit_msg = Message::new_text(EDITED_PREFIX.to_owned() + &new_text); // prefix only set for nicer display in Non-Delta-MUAs
edit_msg.set_quote(context, Some(&original_msg)).await?; // quote only set for nicer display in Non-Delta-MUAs
if original_msg.get_showpadlock() {
edit_msg.param.set_int(Param::GuaranteeE2ee, 1);
}
edit_msg
.param
.set(Param::TextEditFor, original_msg.rfc724_mid);
edit_msg.hidden = true;
send_msg(context, original_msg.chat_id, &mut edit_msg).await?;
Ok(())
}

pub(crate) async fn save_text_edit_to_db(
context: &Context,
original_msg: &mut Message,
new_text: &str,
) -> Result<()> {
original_msg.param.set_int(Param::IsEdited, 1);
context
.sql
.execute(
"UPDATE msgs SET txt=?, txt_normalized=?, param=? WHERE id=?",
(
new_text,
message::normalize_text(new_text),
original_msg.param.to_string(),
original_msg.id,
),
)
.await?;
context.emit_msgs_changed(original_msg.chat_id, original_msg.id);
Ok(())
}

/// Sends invitation to a videochat.
pub async fn send_videochat_invitation(context: &Context, chat_id: ChatId) -> Result<MsgId> {
ensure!(
Expand Down
65 changes: 65 additions & 0 deletions src/chat/chat_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3645,3 +3645,68 @@ async fn test_one_to_one_chat_no_group_member_timestamps() {
let payload = sent.payload;
assert!(!payload.contains("Chat-Group-Member-Timestamps:"));
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_send_edit_request() -> Result<()> {
let mut tcm = TestContextManager::new();
let alice = &tcm.alice().await;
let bob = &tcm.bob().await;
let alice_chat = alice.create_chat(bob).await;

// Alice sends a message with typos, followed by a correction message
let sent1 = alice.send_text(alice_chat.id, "zext me in delra.cat").await;
let alice_msg = sent1.load_from_db().await;
assert_eq!(alice_msg.text, "zext me in delra.cat");

send_edit_request(alice, alice_msg.id, "Text me on Delta.Chat".to_string()).await?;
let sent2 = alice.pop_sent_msg().await;
let test = Message::load_from_db(alice, alice_msg.id).await?;
assert_eq!(test.text, "Text me on Delta.Chat");

// Bob receives both messages and has the correct text at the end
let bob_msg = bob.recv_msg(&sent1).await;
assert_eq!(bob_msg.text, "zext me in delra.cat");

bob.recv_msg_opt(&sent2).await;
let test = Message::load_from_db(bob, bob_msg.id).await?;
assert_eq!(test.text, "Text me on Delta.Chat");

// alice has another device, and sees the correction also there
let alice2 = tcm.alice().await;
let alice2_msg = alice2.recv_msg(&sent1).await;
assert_eq!(alice2_msg.text, "zext me in delra.cat");

alice2.recv_msg_opt(&sent2).await;
let test = Message::load_from_db(&alice2, alice2_msg.id).await?;
assert_eq!(test.text, "Text me on Delta.Chat");

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_receive_edit_request_after_removal() -> Result<()> {
let mut tcm = TestContextManager::new();
let alice = &tcm.alice().await;
let bob = &tcm.bob().await;
let alice_chat = alice.create_chat(bob).await;

// Alice sends a messag with typos, followed by a correction message
let sent1 = alice.send_text(alice_chat.id, "zext me in delra.cat").await;
let alice_msg = sent1.load_from_db().await;
send_edit_request(alice, alice_msg.id, "Text me on Delta.Chat".to_string()).await?;
let sent2 = alice.pop_sent_msg().await;

// Bob receives first message, deletes it and then ignores the correction
let bob_msg = bob.recv_msg(&sent1).await;
let bob_chat_id = bob_msg.chat_id;
assert_eq!(bob_msg.text, "zext me in delra.cat");
assert_eq!(bob_chat_id.get_msg_cnt(bob).await?, 1);

delete_msgs(bob, &[bob_msg.id]).await?;
assert_eq!(bob_chat_id.get_msg_cnt(bob).await?, 0);

bob.recv_msg_trash(&sent2).await;
assert_eq!(bob_chat_id.get_msg_cnt(bob).await?, 0);

Ok(())
}
4 changes: 4 additions & 0 deletions src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ pub(crate) const TIMESTAMP_SENT_TOLERANCE: i64 = 60;
/// on mobile devices. See also [`crate::chat::CantSendReason::SecurejoinWait`].
pub(crate) const SECUREJOIN_WAIT_TIMEOUT: u64 = 15;

// To make text edits clearer for Non-Delta-MUA or old Delta Chats, edited text will be prefixed by EDITED_PREFIX.
// Newer Delta Chats will remove the prefix as needed.
pub(crate) const EDITED_PREFIX: &str = "✏️";

#[cfg(test)]
mod tests {
use num_traits::FromPrimitive;
Expand Down
3 changes: 3 additions & 0 deletions src/headerdef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ pub enum HeaderDef {
ChatDispositionNotificationTo,
ChatWebrtcRoom,

/// This message obsoletes the text of the message defined here by rfc724_mid.
ChatEdit,

/// [Autocrypt](https://autocrypt.org/) header.
Autocrypt,
AutocryptGossip,
Expand Down
5 changes: 5 additions & 0 deletions src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,11 @@ impl Message {
0 != self.param.get_int(Param::Forwarded).unwrap_or_default()
}

/// Returns true if the message is edited.
pub fn is_edited(&self) -> bool {
self.param.get_bool(Param::IsEdited).unwrap_or_default()
}

/// Returns true if the message is an informational message.
pub fn is_info(&self) -> bool {
let cmd = self.param.get_cmd();
Expand Down
14 changes: 13 additions & 1 deletion src/mimefactory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,18 @@ impl MimeFactory {
}
}

if let Loaded::Message { msg, .. } = &self.loaded {
if let Some(original_rfc724_mid) = msg.param.get(Param::TextEditFor) {
headers.push((
"Chat-Edit",
mail_builder::headers::message_id::MessageId::new(
original_rfc724_mid.to_string(),
)
.into(),
));
}
}

// Non-standard headers.
headers.push((
"Chat-Version",
Expand Down Expand Up @@ -849,7 +861,7 @@ impl MimeFactory {
if header_name == "message-id" {
unprotected_headers.push(header.clone());
hidden_headers.push(header.clone());
} else if header_name == "chat-user-avatar" {
} else if header_name == "chat-user-avatar" || header_name == "chat-edit" {
hidden_headers.push(header.clone());
} else if header_name == "autocrypt"
&& !context.get_config_bool(Config::ProtectAutocrypt).await?
Expand Down
5 changes: 4 additions & 1 deletion src/mimeparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,9 @@ impl MimeMessage {

// For now only avatar headers can be hidden.
if !headers.contains_key(&key)
&& (key == "chat-user-avatar" || key == "chat-group-avatar")
&& (key == "chat-user-avatar"
|| key == "chat-group-avatar"
|| key == "chat-edit")
{
headers.insert(key.to_string(), field.get_value());
}
Expand Down Expand Up @@ -448,6 +450,7 @@ impl MimeMessage {
HeaderDef::ChatGroupMemberAdded,
HeaderDef::ChatGroupMemberTimestamps,
HeaderDef::ChatGroupPastMembers,
HeaderDef::ChatEdit,
] {
headers.remove(h.get_headername());
}
Expand Down
7 changes: 6 additions & 1 deletion src/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,12 @@ pub enum Param {

/// For messages: Whether [crate::message::Viewtype::Sticker] should be forced.
ForceSticker = b'X',
// 'L' was defined as ProtectionSettingsTimestamp for Chats, however, never used in production.

/// For messages: Message is a text edit message. the value of this parameter is the rfc724_mid of the original message.
TextEditFor = b'I',

/// For messages: Message text was edited.
IsEdited = b'L',
}

/// An object for handling key=value parameter lists.
Expand Down
37 changes: 36 additions & 1 deletion src/receive_imf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use regex::Regex;
use crate::aheader::EncryptPreference;
use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ProtectionStatus};
use crate::config::Config;
use crate::constants::{Blocked, Chattype, ShowEmails, DC_CHAT_ID_TRASH};
use crate::constants::{Blocked, Chattype, ShowEmails, DC_CHAT_ID_TRASH, EDITED_PREFIX};
use crate::contact::{Contact, ContactId, Origin};
use crate::context::Context;
use crate::debug_logging::maybe_set_logging_xdc_inner;
Expand Down Expand Up @@ -1500,6 +1500,41 @@ async fn add_parts(
}
}

if let Some(rfc724_mid) = mime_parser.get_header(HeaderDef::ChatEdit) {
chat_id = DC_CHAT_ID_TRASH;
if let Some((original_msg_id, _)) = rfc724_mid_exists(context, rfc724_mid).await? {
if let Some(mut original_msg) =
Message::load_from_db_optional(context, original_msg_id).await?
{
if original_msg.from_id == from_id {
if let Some(part) = mime_parser.parts.first() {
let edit_msg_showpadlock = part
.param
.get_bool(Param::GuaranteeE2ee)
.unwrap_or_default();
if edit_msg_showpadlock || !original_msg.get_showpadlock() {
let new_text =
part.msg.strip_prefix(EDITED_PREFIX).unwrap_or(&part.msg);
chat::save_text_edit_to_db(context, &mut original_msg, new_text)
.await?;
} else {
warn!(context, "Edit message: Not encrypted.");
}
}
} else {
warn!(context, "Edit message: Bad sender.");
}
} else {
warn!(context, "Edit message: Database entry does not exist.");
}
} else {
warn!(
context,
"Edit message: rfc724_mid {rfc724_mid:?} not found."
);
}
}

let mut parts = mime_parser.parts.iter().peekable();
while let Some(part) = parts.next() {
if part.is_reaction {
Expand Down

0 comments on commit 85cbfde

Please sign in to comment.