Fix deletion of random SMS when trying to update threads #149
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 tasks
tom93
force-pushed
the
fix-deletion
branch
3 times, most recently
from
68e396e to
91b6c73
Compare
This was referenced Apr 5, 2024
tom93
referenced
this pull request
Apr 5, 2024
https://www.github.com/FossifyOrg/Messages/issues/146 https://stackoverflow.com/questions/6903885/android-programatically-inserted-sms-have-incorrect-timestamp-in-messaging-apps https://android.googlesource.com/platform/packages/providers/TelephonyProvider/+/android14-release/src/com/android/providers/telephony/MmsSmsDatabaseHelper.java#134 https://android.googlesource.com/platform/packages/providers/TelephonyProvider/+/b7577ee9953653333c1ed3523869d95096c1990d%5E%21/src/com/android/providers/telephony/SmsProvider.java
Commit 44c540b (Fixed updating last message after deleting (FossifyOrg#167), 2021-09-04) 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.[1] So instead of deleting the thread with that ID, it deletes whichever SMS happens to have that ID. The fix is to change the condition to an always-false condition, so that no messages will be deleted. The threads will still get updated.[2] Fixes FossifyOrg#148. [1] https://android.googlesource.com/platform/packages/providers/TelephonyProvider/+/android14-release/src/com/android/providers/telephony/MmsSmsProvider.java#1405 [2] https://android.googlesource.com/platform/packages/providers/TelephonyProvider/+/android14-release/src/com/android/providers/telephony/MmsSmsProvider.java#1409
|
Seems to work, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit 44c540b 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.
The fix is to change the condition to an always-false condition, so that no messages will be deleted. The threads will still get
updated.
Old updates
Update 1: A minor issue with this PR is that the conversation dates don't get updated any more (the date should be updated to the last non-deleted message in the conversation, but that doesn't happen). I'm not sure why that is.
Update 2: I think the Telephony database is getting updated correctly, but the app's cache isn't getting updated (probably because I removed the call to insertOrUpdateConversation()). I'm looking into this, but I think this it's still worth merging and releasing as-is because deleting random messages is not good!
Update 3: I restored the call to insertOrUpdateConversation() and everything seems fine now. I squashed and force-pushed.
What is it?
Description of the changes in your PR
Before/After Screenshots/Screen Record
See #148.
Fixes the following issue(s)
Acknowledgement