Deleting one message causes another arbitrary message to be deleted

[tom93]

Proposed fix: #149.

Checklist

• [X] I can reproduce the bug with the latest version given here.
• [X] I made sure that there are no existing issues - open or closed - to which I could contribute my information.
• [X] I made sure that there are no existing discussions - open or closed - to which I could contribute my information.
• [X] I have read the FAQs inside the app (Menu -> About -> FAQs) and my problem isn't listed.
• [X] I have taken the time to fill in all the required details. I understand that the bug report will be dismissed otherwise.
• [X] This issue contains only one bug.
• [X] I have read and understood the contribution guidelines.

Affected app version

1.0.1

Affected Android/Custom ROM version

Android 14

Affected device model

Emulator

How did you install the app?

GitHub releases

Steps to reproduce the bug

(See screen recording for demo)

1. Use a clean phone with no messages to avoid deleting important messages and to be able to reproduce this reliably.
2. Create a conversation with 555-0101 and send two SMS's: "Message 1" and "Message 2".
3. Create a conversation with 555-0102 and send an SMS: "Message 3".
4. Delete "Message 3".
5. Switch to the conversation with 555-0101.

Expected behavior

There should still be two SMS's there ("Message 1" and "Message 2").

Actual behavior

"Message 2" is deleted (!)

Screenshots/Screen recordings

https://github.com/FossifyOrg/Messages/assets/5887562/9833d435-5c2c-4bb9-8f62-a8a07d90d433

(the end got clipped a little, you may need to pause to see that Message 2 disappears)

Additional information

Workaround: DO NOT DELETE MESSAGES

Cause: Commit 44c540b96187d02435b4ad2eccfbace1e2116a27 introduced a hack to update the conversation snippet, however there is a bug in that code: it tries to delete by thread ID, but the condition is applied to the SMS table, not to the threads table (Android source). So instead of deleting the thread with that ID, it deletes whichever SMS happens to have that ID.

Proposed solution: I haven't tested this yet, but I believe changing the condition to "1=0" will fix the issue (it will still cause the threads to be updated but won't delete anything). Update: I've tested it using the steps above and it seems to work fine, but additional testing would be welcome.

CC: @Aga-C

[tom93]

It might be possible to recover unintentionally-deleted messages using the Room DAO cache, I'm not sure how easy/reliable that would be.

Either way, I think my proposed fix (#149) should be merged and released ASAP (I don't think it will interfere with potential future recovery efforts).

[naveensingh]

@Aga-C have you tried reproducing this bug?

[Aga-C]

@naveensingh I was able to reproduce it after the fresh install of the app with no SMS messages on the phone yet. With more messages, maybe either I didn't notice or the IDs were already so high that nothing got removed.

I've also checked, no one reported it in Simple SMS Messenger, but the code has been there since 2021.

[tom93]

With more messages, maybe either I didn't notice or the IDs were already so high that nothing got removed.

For reference, when there are many messages, the behaviour I'm predicting is that if you delete a message from a conversation whose ID is i (meaning, the i-th oldest conversation by date of first message), then it will also delete the message with ID i (meaning the i-th oldest message across all conversations). So it will be the oldest messages that get unintentionally deleted, and the unintentional deletion will only occur for the first deletion in each conversation.

To maximise the chances of discovery, the process would be:

1. Make a note of the oldest messages (by going over the conversations and keeping track of the oldest messages you find).
2. Delete one message from each conversation (e.g. the most recent message).
3. Check if the oldest messages still exist. If you have n conversations and you've never deleted any messages before this test, then I predict that the oldest n messages will be unintentionally deleted. If you deleted messages in the past, then there will be fewer unintentional deletions from step 2, because some of those unintentional deletions would have already occurred in the past! Also, note that ID's might not correspond to age for imported messages.

Obviously back up the messages first. Personally I wouldn't attempt this on a device with important messages, even with a backup.

(Edited to clarify)