Skip to content

Commit 4dbbd4d

Browse files
committed
feat: remove non-sticker heuristics and force_sticker()
This causes problems for Delta Chat Desktop at <deltachat/deltachat-desktop#6278> even though the logic was originally introduced for iOS. If the problem remains on iOS, heuristics can be added into iOS UI.
1 parent d069e75 commit 4dbbd4d

8 files changed

Lines changed: 43 additions & 76 deletions

File tree

deltachat-jsonrpc/src/api.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2379,9 +2379,6 @@ impl CommandApi {
23792379
let mut msg = Message::new(Viewtype::Sticker);
23802380
msg.set_file_and_deduplicate(&ctx, Path::new(&sticker_path), None, None)?;
23812381

2382-
// JSON-rpc does not need heuristics to turn [Viewtype::Sticker] into [Viewtype::Image]
2383-
msg.force_sticker();
2384-
23852382
let message_id = deltachat::chat::send_msg(&ctx, ChatId::new(chat_id), &mut msg).await?;
23862383
Ok(message_id.to_u32())
23872384
}

deltachat-jsonrpc/src/api/types/message.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,6 @@ pub enum MessageViewtype {
287287
Gif,
288288

289289
/// Message containing a sticker, similar to image.
290-
/// NB: When sending, the message viewtype may be changed to `Image` by some heuristics like
291-
/// checking for transparent pixels. Use `Message::force_sticker()` to disable them.
292290
///
293291
/// If possible, the ui should display the image without borders in a transparent way.
294292
/// A click on a sticker will offer to install the sticker set in some future.

src/blob.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,6 @@ impl<'a> BlobObject<'a> {
284284
///
285285
/// Recoding is only done for [`Viewtype::Image`]. For [`Viewtype::File`], if it's a correct
286286
/// image, `*viewtype` is set to [`Viewtype::Image`].
287-
///
288-
/// On some platforms images are passed to Core as [`Viewtype::Sticker`]. We recheck if the
289-
/// image is a true sticker assuming that it must have at least one fully transparent corner,
290-
/// otherwise `*viewtype` is set to [`Viewtype::Image`].
291287
pub async fn check_or_recode_image(
292288
&mut self,
293289
context: &Context,

src/blob/blob_tests.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,14 +445,14 @@ async fn test_recode_image_balanced_png() {
445445
.await
446446
.unwrap();
447447

448-
// This will be sent as Image, see [`BlobObject::check_or_recode_image()`] for explanation.
449448
SendImageCheckMediaquality {
450449
viewtype: Viewtype::Sticker,
451450
media_quality_config: "0",
452451
bytes,
453452
extension: "png",
454453
original_width: 1920,
455454
original_height: 1080,
455+
res_viewtype: Some(Viewtype::Sticker),
456456
compressed_width: 1920,
457457
compressed_height: 1080,
458458
..Default::default()
@@ -734,8 +734,6 @@ async fn test_send_gif_as_sticker() -> Result<()> {
734734
let chat = alice.get_self_chat().await;
735735
let sent = alice.send_msg(chat.id, &mut msg).await;
736736
let msg = Message::load_from_db(alice, sent.sender_msg_id).await?;
737-
// Message::force_sticker() wasn't used, still Viewtype::Sticker is preserved because of the
738-
// extension.
739737
assert_eq!(msg.get_viewtype(), Viewtype::Sticker);
740738
Ok(())
741739
}

src/chat.rs

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2467,23 +2467,15 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
24672467
.with_context(|| format!("attachment missing for message of type #{}", msg.viewtype))?;
24682468
let mut maybe_image = false;
24692469

2470-
if msg.viewtype == Viewtype::File
2471-
|| msg.viewtype == Viewtype::Image
2472-
|| msg.viewtype == Viewtype::Sticker && !msg.param.exists(Param::ForceSticker)
2473-
{
2470+
if msg.viewtype == Viewtype::File || msg.viewtype == Viewtype::Image {
24742471
// Correct the type, take care not to correct already very special
24752472
// formats as GIF or VOICE.
24762473
//
24772474
// Typical conversions:
24782475
// - from FILE to AUDIO/VIDEO/IMAGE
24792476
// - from FILE/IMAGE to GIF */
24802477
if let Some((better_type, _)) = message::guess_msgtype_from_suffix(msg) {
2481-
if msg.viewtype == Viewtype::Sticker {
2482-
if better_type != Viewtype::Image {
2483-
// UIs don't want conversions of `Sticker` to anything other than `Image`.
2484-
msg.param.set_int(Param::ForceSticker, 1);
2485-
}
2486-
} else if better_type == Viewtype::Image {
2478+
if better_type == Viewtype::Image {
24872479
maybe_image = true;
24882480
} else if better_type != Viewtype::Webxdc
24892481
|| context
@@ -2503,10 +2495,7 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
25032495
if msg.viewtype == Viewtype::Vcard {
25042496
msg.try_set_vcard(context, &blob.to_abs_path()).await?;
25052497
}
2506-
if msg.viewtype == Viewtype::File && maybe_image
2507-
|| msg.viewtype == Viewtype::Image
2508-
|| msg.viewtype == Viewtype::Sticker && !msg.param.exists(Param::ForceSticker)
2509-
{
2498+
if msg.viewtype == Viewtype::File && maybe_image || msg.viewtype == Viewtype::Image {
25102499
let new_name = blob
25112500
.check_or_recode_image(context, msg.get_filename(), &mut msg.viewtype)
25122501
.await?;

src/chat/chat_tests.rs

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2075,13 +2075,7 @@ async fn test_chat_get_color_encrypted() -> Result<()> {
20752075
Ok(())
20762076
}
20772077

2078-
async fn test_sticker(
2079-
filename: &str,
2080-
bytes: &[u8],
2081-
res_viewtype: Viewtype,
2082-
w: i32,
2083-
h: i32,
2084-
) -> Result<()> {
2078+
async fn test_sticker(filename: &str, bytes: &[u8], w: i32, h: i32) -> Result<()> {
20852079
let alice = TestContext::new_alice().await;
20862080
let bob = TestContext::new_bob().await;
20872081
let alice_chat = alice.create_chat(&bob).await;
@@ -2097,7 +2091,7 @@ async fn test_sticker(
20972091

20982092
let msg = bob.recv_msg(&sent_msg).await;
20992093
assert_eq!(msg.chat_id, bob_chat.id);
2100-
assert_eq!(msg.get_viewtype(), res_viewtype);
2094+
assert_eq!(msg.get_viewtype(), Viewtype::Sticker);
21012095
assert_eq!(msg.get_filename().unwrap(), filename);
21022096
assert_eq!(msg.get_width(), w);
21032097
assert_eq!(msg.get_height(), h);
@@ -2111,7 +2105,6 @@ async fn test_sticker_png() -> Result<()> {
21112105
test_sticker(
21122106
"sticker.png",
21132107
include_bytes!("../../test-data/image/logo.png"),
2114-
Viewtype::Sticker,
21152108
135,
21162109
135,
21172110
)
@@ -2123,18 +2116,40 @@ async fn test_sticker_jpeg() -> Result<()> {
21232116
test_sticker(
21242117
"sticker.jpg",
21252118
include_bytes!("../../test-data/image/avatar1000x1000.jpg"),
2126-
Viewtype::Image,
21272119
1000,
21282120
1000,
21292121
)
21302122
.await
21312123
}
21322124

21332125
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2134-
async fn test_sticker_jpeg_force() {
2135-
let alice = TestContext::new_alice().await;
2136-
let bob = TestContext::new_bob().await;
2137-
let alice_chat = alice.create_chat(&bob).await;
2126+
async fn test_sticker_gif() -> Result<()> {
2127+
test_sticker(
2128+
"sticker.gif",
2129+
include_bytes!("../../test-data/image/logo.gif"),
2130+
135,
2131+
135,
2132+
)
2133+
.await
2134+
}
2135+
2136+
/// Tests that stickers are sent as stickers.
2137+
///
2138+
/// Previously there was heuristic that stickers
2139+
/// were sometimes turned into non-stickers,
2140+
/// e.g. when it looked like UI sent
2141+
/// a screenshot dragged from the gallery into chat
2142+
/// as a sticker.
2143+
///
2144+
/// We have no such heuristic anymore,
2145+
/// if such heuristic is needed on some platform,
2146+
/// UI code should implement it.
2147+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2148+
async fn test_sticker_no_heuristics() {
2149+
let mut tcm = TestContextManager::new();
2150+
let alice = &tcm.alice().await;
2151+
let bob = &tcm.bob().await;
2152+
let alice_chat = alice.create_chat(bob).await;
21382153

21392154
let file = alice.get_blobdir().join("sticker.jpg");
21402155
tokio::fs::write(
@@ -2144,53 +2159,38 @@ async fn test_sticker_jpeg_force() {
21442159
.await
21452160
.unwrap();
21462161

2147-
// Images without force_sticker should be turned into [Viewtype::Image]
2162+
// Send a sticker.
21482163
let mut msg = Message::new(Viewtype::Sticker);
2149-
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)
2164+
msg.set_file_and_deduplicate(alice, &file, Some("sticker.jpg"), None)
21502165
.unwrap();
2151-
let file = msg.get_file(&alice).unwrap();
2166+
let file = msg.get_file(alice).unwrap();
21522167
let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await;
21532168
let msg = bob.recv_msg(&sent_msg).await;
2154-
assert_eq!(msg.get_viewtype(), Viewtype::Image);
2169+
assert_eq!(msg.get_viewtype(), Viewtype::Sticker);
21552170

2156-
// Images with `force_sticker = true` should keep [Viewtype::Sticker]
2171+
// Send a sticker reusing the file.
21572172
let mut msg = Message::new(Viewtype::Sticker);
2158-
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)
2173+
msg.set_file_and_deduplicate(alice, &file, Some("sticker.jpg"), None)
21592174
.unwrap();
2160-
msg.force_sticker();
21612175
let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await;
21622176
let msg = bob.recv_msg(&sent_msg).await;
21632177
assert_eq!(msg.get_viewtype(), Viewtype::Sticker);
21642178

2165-
// Images with `force_sticker = true` should keep [Viewtype::Sticker]
2166-
// even on drafted messages
2179+
// Set sticker as a draft, then send it.
21672180
let mut msg = Message::new(Viewtype::Sticker);
2168-
msg.set_file_and_deduplicate(&alice, &file, Some("sticker.jpg"), None)
2181+
msg.set_file_and_deduplicate(alice, &file, Some("sticker.jpg"), None)
21692182
.unwrap();
2170-
msg.force_sticker();
21712183
alice_chat
21722184
.id
2173-
.set_draft(&alice, Some(&mut msg))
2185+
.set_draft(alice, Some(&mut msg))
21742186
.await
21752187
.unwrap();
2176-
let mut msg = alice_chat.id.get_draft(&alice).await.unwrap().unwrap();
2188+
let mut msg = alice_chat.id.get_draft(alice).await.unwrap().unwrap();
21772189
let sent_msg = alice.send_msg(alice_chat.id, &mut msg).await;
21782190
let msg = bob.recv_msg(&sent_msg).await;
21792191
assert_eq!(msg.get_viewtype(), Viewtype::Sticker);
21802192
}
21812193

2182-
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2183-
async fn test_sticker_gif() -> Result<()> {
2184-
test_sticker(
2185-
"sticker.gif",
2186-
include_bytes!("../../test-data/image/logo.gif"),
2187-
Viewtype::Sticker,
2188-
135,
2189-
135,
2190-
)
2191-
.await
2192-
}
2193-
21942194
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
21952195
async fn test_sticker_forward() -> Result<()> {
21962196
// create chats

src/message.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -795,12 +795,6 @@ impl Message {
795795
self.viewtype
796796
}
797797

798-
/// Forces the message to **keep** [Viewtype::Sticker]
799-
/// e.g the message will not be converted to a [Viewtype::Image].
800-
pub fn force_sticker(&mut self) {
801-
self.param.set_int(Param::ForceSticker, 1);
802-
}
803-
804798
/// Returns the state of the message.
805799
pub fn get_state(&self) -> MessageState {
806800
self.state
@@ -2322,8 +2316,6 @@ pub enum Viewtype {
23222316
Gif = 21,
23232317

23242318
/// Message containing a sticker, similar to image.
2325-
/// NB: When sending, the message viewtype may be changed to `Image` by some heuristics like
2326-
/// checking for transparent pixels. Use `Message::force_sticker()` to disable them.
23272319
///
23282320
/// If possible, the ui should display the image without borders in a transparent way.
23292321
/// A click on a sticker will offer to install the sticker set in some future.

src/param.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,6 @@ pub enum Param {
247247
/// For Webxdc Message Instances: Chat to integrate the Webxdc for.
248248
WebxdcIntegrateFor = b'2',
249249

250-
/// For messages: Whether [crate::message::Viewtype::Sticker] should be forced.
251-
ForceSticker = b'X',
252-
253250
/// For messages: Message is a deletion request. The value is a list of rfc724_mid of the messages to delete.
254251
DeleteRequestFor = b'M',
255252

0 commit comments

Comments
 (0)