From 3aeb2d44b75827612d44c3fc504b17bdeec5ed60 Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 24 Apr 2026 17:34:30 +0200 Subject: [PATCH] feat: remove non-sticker heuristics and force_sticker() This causes problems for Delta Chat Desktop at even though the logic was originally introduced for iOS. If the problem remains on iOS, heuristics can be added into iOS UI. --- deltachat-jsonrpc/src/api.rs | 3 - deltachat-jsonrpc/src/api/types/message.rs | 2 - src/blob.rs | 4 - src/blob/blob_tests.rs | 4 +- src/chat.rs | 17 +---- src/chat/chat_tests.rs | 88 +++++++++++----------- src/message.rs | 8 -- src/param.rs | 3 - 8 files changed, 48 insertions(+), 81 deletions(-) diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index dc4842fc7..8037c9afd 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -2379,9 +2379,6 @@ impl CommandApi { let mut msg = Message::new(Viewtype::Sticker); msg.set_file_and_deduplicate(&ctx, Path::new(&sticker_path), None, None)?; - // JSON-rpc does not need heuristics to turn [Viewtype::Sticker] into [Viewtype::Image] - msg.force_sticker(); - let message_id = deltachat::chat::send_msg(&ctx, ChatId::new(chat_id), &mut msg).await?; Ok(message_id.to_u32()) } diff --git a/deltachat-jsonrpc/src/api/types/message.rs b/deltachat-jsonrpc/src/api/types/message.rs index 7423c5aa4..2b37fb507 100644 --- a/deltachat-jsonrpc/src/api/types/message.rs +++ b/deltachat-jsonrpc/src/api/types/message.rs @@ -287,8 +287,6 @@ pub enum MessageViewtype { Gif, /// Message containing a sticker, similar to image. - /// NB: When sending, the message viewtype may be changed to `Image` by some heuristics like - /// checking for transparent pixels. Use `Message::force_sticker()` to disable them. /// /// If possible, the ui should display the image without borders in a transparent way. /// A click on a sticker will offer to install the sticker set in some future. diff --git a/src/blob.rs b/src/blob.rs index ac075a1fd..d8a5eac9d 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -284,10 +284,6 @@ impl<'a> BlobObject<'a> { /// /// Recoding is only done for [`Viewtype::Image`]. For [`Viewtype::File`], if it's a correct /// image, `*viewtype` is set to [`Viewtype::Image`]. - /// - /// On some platforms images are passed to Core as [`Viewtype::Sticker`]. We recheck if the - /// image is a true sticker assuming that it must have at least one fully transparent corner, - /// otherwise `*viewtype` is set to [`Viewtype::Image`]. pub async fn check_or_recode_image( &mut self, context: &Context, diff --git a/src/blob/blob_tests.rs b/src/blob/blob_tests.rs index 1e9fbc720..014a2cce4 100644 --- a/src/blob/blob_tests.rs +++ b/src/blob/blob_tests.rs @@ -445,7 +445,6 @@ async fn test_recode_image_balanced_png() { .await .unwrap(); - // This will be sent as Image, see [`BlobObject::check_or_recode_image()`] for explanation. SendImageCheckMediaquality { viewtype: Viewtype::Sticker, media_quality_config: "0", @@ -453,6 +452,7 @@ async fn test_recode_image_balanced_png() { extension: "png", original_width: 1920, original_height: 1080, + res_viewtype: Some(Viewtype::Sticker), compressed_width: 1920, compressed_height: 1080, ..Default::default() @@ -734,8 +734,6 @@ async fn test_send_gif_as_sticker() -> Result<()> { let chat = alice.get_self_chat().await; let sent = alice.send_msg(chat.id, &mut msg).await; let msg = Message::load_from_db(alice, sent.sender_msg_id).await?; - // Message::force_sticker() wasn't used, still Viewtype::Sticker is preserved because of the - // extension. assert_eq!(msg.get_viewtype(), Viewtype::Sticker); Ok(()) } diff --git a/src/chat.rs b/src/chat.rs index ed1de6a89..f5a8d8328 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2467,10 +2467,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { .with_context(|| format!("attachment missing for message of type #{}", msg.viewtype))?; let mut maybe_image = false; - if msg.viewtype == Viewtype::File - || msg.viewtype == Viewtype::Image - || msg.viewtype == Viewtype::Sticker && !msg.param.exists(Param::ForceSticker) - { + if msg.viewtype == Viewtype::File || msg.viewtype == Viewtype::Image { // Correct the type, take care not to correct already very special // formats as GIF or VOICE. // @@ -2478,12 +2475,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { // - from FILE to AUDIO/VIDEO/IMAGE // - from FILE/IMAGE to GIF */ if let Some((better_type, _)) = message::guess_msgtype_from_suffix(msg) { - if msg.viewtype == Viewtype::Sticker { - if better_type != Viewtype::Image { - // UIs don't want conversions of `Sticker` to anything other than `Image`. - msg.param.set_int(Param::ForceSticker, 1); - } - } else if better_type == Viewtype::Image { + if better_type == Viewtype::Image { maybe_image = true; } else if better_type != Viewtype::Webxdc || context @@ -2503,10 +2495,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { if msg.viewtype == Viewtype::Vcard { msg.try_set_vcard(context, &blob.to_abs_path()).await?; } - if msg.viewtype == Viewtype::File && maybe_image - || msg.viewtype == Viewtype::Image - || msg.viewtype == Viewtype::Sticker && !msg.param.exists(Param::ForceSticker) - { + if msg.viewtype == Viewtype::File && maybe_image || msg.viewtype == Viewtype::Image { let new_name = blob .check_or_recode_image(context, msg.get_filename(), &mut msg.viewtype) .await?; diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index 36cf276d7..3e15245b1 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2075,13 +2075,7 @@ async fn test_chat_get_color_encrypted() -> Result<()> { Ok(()) } -async fn test_sticker( - filename: &str, - bytes: &[u8], - res_viewtype: Viewtype, - w: i32, - h: i32, -) -> Result<()> { +async fn test_sticker(filename: &str, bytes: &[u8], w: i32, h: i32) -> Result<()> { let alice = TestContext::new_alice().await; let bob = TestContext::new_bob().await; let alice_chat = alice.create_chat(&bob).await; @@ -2097,7 +2091,7 @@ async fn test_sticker( let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.chat_id, bob_chat.id); - assert_eq!(msg.get_viewtype(), res_viewtype); + assert_eq!(msg.get_viewtype(), Viewtype::Sticker); assert_eq!(msg.get_filename().unwrap(), filename); assert_eq!(msg.get_width(), w); assert_eq!(msg.get_height(), h); @@ -2111,7 +2105,6 @@ async fn test_sticker_png() -> Result<()> { test_sticker( "sticker.png", include_bytes!("../../test-data/image/logo.png"), - Viewtype::Sticker, 135, 135, ) @@ -2123,7 +2116,6 @@ async fn test_sticker_jpeg() -> Result<()> { test_sticker( "sticker.jpg", include_bytes!("../../test-data/image/avatar1000x1000.jpg"), - Viewtype::Image, 1000, 1000, ) @@ -2131,10 +2123,33 @@ async fn test_sticker_jpeg() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_sticker_jpeg_force() { - let alice = TestContext::new_alice().await; - let bob = TestContext::new_bob().await; - let alice_chat = alice.create_chat(&bob).await; +async fn test_sticker_gif() -> Result<()> { + test_sticker( + "sticker.gif", + include_bytes!("../../test-data/image/logo.gif"), + 135, + 135, + ) + .await +} + +/// Tests that stickers are sent as stickers. +/// +/// Previously there was heuristic that stickers +/// were sometimes turned into non-stickers, +/// e.g. when it looked like UI sent +/// a screenshot dragged from the gallery into chat +/// as a sticker. +/// +/// We have no such heuristic anymore, +/// if such heuristic is needed on some platform, +/// UI code should implement it. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_sticker_no_heuristics() { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let alice_chat = alice.create_chat(bob).await; let file = alice.get_blobdir().join("sticker.jpg"); tokio::fs::write( @@ -2144,53 +2159,38 @@ async fn test_sticker_jpeg_force() { .await .unwrap(); - // Images without force_sticker should be turned into [Viewtype::Image] + // Send a sticker. let mut msg = Message::new(Viewtype::Sticker); - msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None) + msg.set_file_and_deduplicate(alice, &file, Some("sticker.jpg"), None) .unwrap(); - let file = msg.get_file(&alice).unwrap(); - let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; - let msg = bob.recv_msg(&sent_msg).await; - assert_eq!(msg.get_viewtype(), Viewtype::Image); - - // Images with `force_sticker = true` should keep [Viewtype::Sticker] - let mut msg = Message::new(Viewtype::Sticker); - msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None) - .unwrap(); - msg.force_sticker(); + let file = msg.get_file(alice).unwrap(); let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.get_viewtype(), Viewtype::Sticker); - // Images with `force_sticker = true` should keep [Viewtype::Sticker] - // even on drafted messages + // Send a sticker reusing the file. let mut msg = Message::new(Viewtype::Sticker); - msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None) + msg.set_file_and_deduplicate(alice, &file, Some("sticker.jpg"), None) + .unwrap(); + let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; + let msg = bob.recv_msg(&sent_msg).await; + assert_eq!(msg.get_viewtype(), Viewtype::Sticker); + + // Set sticker as a draft, then send it. + let mut msg = Message::new(Viewtype::Sticker); + msg.set_file_and_deduplicate(alice, &file, Some("sticker.jpg"), None) .unwrap(); - msg.force_sticker(); alice_chat .id - .set_draft(&alice, Some(&mut msg)) + .set_draft(alice, Some(&mut msg)) .await .unwrap(); - let mut msg = alice_chat.id.get_draft(&alice).await.unwrap().unwrap(); + let mut msg = alice_chat.id.get_draft(alice).await.unwrap().unwrap(); let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await; let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.get_viewtype(), Viewtype::Sticker); } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_sticker_gif() -> Result<()> { - test_sticker( - "sticker.gif", - include_bytes!("../../test-data/image/logo.gif"), - Viewtype::Sticker, - 135, - 135, - ) - .await -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_sticker_forward() -> Result<()> { // create chats diff --git a/src/message.rs b/src/message.rs index d40de7299..27a8e97f8 100644 --- a/src/message.rs +++ b/src/message.rs @@ -795,12 +795,6 @@ impl Message { self.viewtype } - /// Forces the message to **keep** [Viewtype::Sticker] - /// e.g the message will not be converted to a [Viewtype::Image]. - pub fn force_sticker(&mut self) { - self.param.set_int(Param::ForceSticker, 1); - } - /// Returns the state of the message. pub fn get_state(&self) -> MessageState { self.state @@ -2322,8 +2316,6 @@ pub enum Viewtype { Gif = 21, /// Message containing a sticker, similar to image. - /// NB: When sending, the message viewtype may be changed to `Image` by some heuristics like - /// checking for transparent pixels. Use `Message::force_sticker()` to disable them. /// /// If possible, the ui should display the image without borders in a transparent way. /// A click on a sticker will offer to install the sticker set in some future. diff --git a/src/param.rs b/src/param.rs index 087bba93e..7f6b3fa48 100644 --- a/src/param.rs +++ b/src/param.rs @@ -247,9 +247,6 @@ pub enum Param { /// For Webxdc Message Instances: Chat to integrate the Webxdc for. WebxdcIntegrateFor = b'2', - /// For messages: Whether [crate::message::Viewtype::Sticker] should be forced. - ForceSticker = b'X', - /// For messages: Message is a deletion request. The value is a list of rfc724_mid of the messages to delete. DeleteRequestFor = b'M',