Feature: File transfer in secure conversations #231
Conversation
fix for secure inc mms, copy headers from original
Mms size and imageresize option
…ithub.com:vdveer/SMSSecure into experimental_multimms_file_transfer_in_securemode
…_in_securemode Experimental mms file transfer in securemode
| @@ -970,6 +1022,7 @@ private Drafts getDraftsForCurrentState() { | |||
| for (Slide slide : attachmentManager.getSlideDeck().getSlides()) { | |||
| if (slide.hasAudio()) drafts.add(new Draft(Draft.AUDIO, slide.getUri().toString())); | |||
| else if (slide.hasVideo()) drafts.add(new Draft(Draft.VIDEO, slide.getUri().toString())); | |||
| else if (slide.hasFile()) drafts.add(new Draft(Draft.FILE, slide.getUri().toString())); | |||
There was a problem hiding this comment.
This line of code can use some comment; because the order here is pretty important. hasImage() needs to be the last one, as it is also used to determine the fact if a thumbnail needs to be shown (hasImage is true on audio and file).
|
Looks great. You need to:
I didn't test your branch (will do after a rebase), but if I read your code well, you use SMS to send files, right? If so, what's the relation to #226? |
|
I have merged master of my upstream origin (equals smssecure/master) into my branch but I will do that again if I missed any commits and squash the commits into one. The file slides are transmitted through mms, not sms. There is no relation to #226 only that I reverted those commits on this branch (my master- and featurebranch allows setting the size), so when squashed there will be no relation at all. I have some questions on the tests, how can I test my code in secure mode in the simulator? I used my (luckily unlimited) subscription to test this feature but it takes too much time to do so. As I am new to this project and Android development any tips or links to tips are appreciated. Thanks for your feedback and managing this repo! |
|
As I am working on multipart mms for filetransfer I would also like to open a discussion here on this matter. Is this something we want to include? Send a file of for example 3mb over a 280kb limited transport channel will potentially make it an expensive feature. On my local branch I only made it available if a preference is set to accept the feature and a notice is shown to the user that multiple (including the amount of) mms messages will be sent when attaching the oversized file to the message. For me this is a perfect feature, since I have unlimited EU MMS (and for example regular data on a internet apn I have to pay for while abroad in EU) , but I am not sure if this can be useful to the majority of users. What do you think? Just the be sure; the multipart feature is not included in this pullrequest. |
|
To test a build, just run Showing the number of MMSs is a good solution. As I said in #226, increasing MMS limit will brake compatibility on some networks. |
|
About the mms size, that is indeed why I reverted these commits (mmssize is static again is this PR) and only leave them in my local branches. In this PR only files <280kB are attachable; others are discarded with a MediaTooLargeException (which lead to a toast telling the user the file is too large) like images when the residing and compressing fails to compress them below 280kB. |
|
OK. Can you rebase and squash your commits for now? |
|
I will but haven't found myself near a computer yet. |
|
I can seem to squash the commits on this branch so I have set up test_pr, which I set to upstream/master and merged my featurebranch. Reset to upstream master and squashed the changes into a single commit. I will close this PR and open a new one from this branch as I can't seem to edit this one. |
|
You can force push to |
Opened a PR because I am not sure if you read posts on closed PR's to discuss the usefulness of such feature. It works pretty well as I believe, including the edgecases when users try to attach and then switch transport mode. Filename addition to de PDU was also a nice one to debug. Well, let me know.