-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a possibility to delete downloaded media to free up storage #1189
Conversation
Thanks! This is awesome |
Would be really great to see this in the app store build sometime. Slowly running out of disk space on my device :). If there are any problems with the feature, just let me know. |
@stigger Definitely! I am planning on including it in the next release :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stigger This is a great start! Before merging we should fix the following crash:
- Send/receive some media messages
- Go to Storage Management and delete media for that contact
- Go back to conversation
The crash is related to an invalid layout, but I think the underlying issue is that there are now media messages that no longer have a corresponding OTRMediaItem
. I think we'd at least want some placeholder to appear where the old media message used to be.
Btw here's a fix for the YapDatabase 4.0 migration: 772d333
I tested this scenario extensively, but only in a simulator. Does it crash for you in the simulator or on a device? Can you show me the crashlog, so I know, what exactly to look for? |
On the simulator, iPhone 11 Pro Max 13.4 using Xcode 11.4. The crash is coming from inside JSQMessagesVC during To clarify the above repro steps, I didn't delete the media messages from the conversation itself before going to storage management. So after clearing via storage management, there are some messages that are missing their underlying media data. |
Yeah, that's how I understood. That's exactly the use-case I wanted to cover, and tested it many times: after deleting the media, placeholders should appear instead of messages, as if the automatic downloading is disabled. You should be able to download the media again, and repeat this cycle many times. Never seen any crashes like what you're describing. I'll try to reproduce some more this weekend. |
I tried to reproduce, but couldn't, I get the expected placeholders instead of the media, no crashes. Could it be that this was some unrelated crash? |
@stigger: Any news on it? |
@stigger After testing again this evening I couldn't reproduce the crash. 🤷♂ However I don't see a proper placeholder for re-downloading the media again for either OMEMO or plaintext messages. I don't necessarily expect OMEMO media messages to be re-downloaded, but plaintext ones should in theory still work. This might be a bug beyond the scope of your PR, although I haven't seen this particular layout issue before. I'm testing with your changes merged into master here if that helps: /~https://github.com/ChatSecure/ChatSecure-iOS/tree/stigger-storage_management |
Nice! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must have been some other layout bug, after further testing I think this is good to merge! Thank you!
No description provided.