Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable media message forwarding #3311

Closed
wants to merge 1 commit into from
Closed

Enable media message forwarding #3311

wants to merge 1 commit into from

Conversation

McLoo
Copy link
Contributor

@McLoo McLoo commented Jun 2, 2015

This is a stew mainly of #2886 with some nice chunks of #2236

  • Tested forwarding to existing as well as new (group-)convos.
  • Tested on a 5.0.2 device and KK emulator. Couldn't get a play service enabled GB emu - can't use a new TS installation without play services... 馃槥

All media works fine as well as text and text + media.

@jeremymasters
Copy link

Does this also fix selection of media messages? In 2.16.3 I can't long-press the image to select it. I have to long-press in the open space beside it.

@2-4601
Copy link
Contributor

2-4601 commented Jun 2, 2015

@jeremymasters See: #2333, #2488, #2648 (PR).

@rhodey
Copy link
Contributor

rhodey commented Jun 12, 2015

hey @McLoo, I just checked out this PR, rebased it with master, and ran some tests. every time i try to forward an image message the image preview is the red error icon, tapping send results in no image being sent.

can you rebase this and let me know if you can reproduce? I tested this on my Nexus 5 running stock Android 5.1 Lollipop, rebased into f72cd5b

@McLoo
Copy link
Contributor Author

McLoo commented Jun 12, 2015

@rhodey then you collabs broke it 馃槈

Will check out. The magic is in those lines:

if (isAuthorityPartDb(uri))
  part.setDataUri(PartAuthority.getPublicPartUri(uri));
else
  part.setDataUri(uri);

Had the triangle before, when not using getPublicPartUri?!

Edit: this occurred only for images...

@McLoo
Copy link
Contributor Author

McLoo commented Jun 13, 2015

@rhodey tested on CM 12.1 with 2.18.1
Still works for me. Can you post a debug log? 馃榿

@rhodey
Copy link
Contributor

rhodey commented Jun 15, 2015

hey @McLoo, I just rebased this with 2.18.1 and tested on my stock 2.3.6 Gingerbread device & my stock 4.4.2 device, both work great. my nexus 5 is my daily use phone, I keep my actual install of TextSecure on there, so when I want to test a new build of TS on that device I have to alter the application ID and some other package names in the build.grade and AndroidManifest.xml file-- basically the app that I test on this device is not the exact app that I should be testing.

so, in conclusion it probably doesn't work on my nexus 5 for reasons related to me changing the application ID and package names. interestingly nothing was actually caught in the log file related to the error icon showing up. this PR looks good to me and has tested well, if we could get someone to test this on a stock lollipop device I'd be super confident in merging it.

馃憤

@McLoo
Copy link
Contributor Author

McLoo commented Jun 16, 2015

I'm thinking of assertMediaSize everytime - not only if not isAuthorityPartDb.
There could be changes for media size, after the media went into the DB?!

(assertMediaSize should probably move into sendMediaMessage, once bigger files are allowed for push)

@McLoo
Copy link
Contributor Author

McLoo commented Jun 16, 2015

Finally I found someone who was willing to try this PR on his daily use phone. 馃槵
Titanium Backup works great if you have to juggle with builds while keeping data.

So: on my Sony Z1 compact 5.0.2 (rooted) this also works fine. Couldn't test media playback though cause of #3436 works with PR 3437

@@ -17,7 +17,7 @@

private static final String PART_URI_STRING = "content://org.thoughtcrime.securesms/part";
private static final String THUMB_URI_STRING = "content://org.thoughtcrime.securesms/thumb";
private static final Uri PART_CONTENT_URI = Uri.parse(PART_URI_STRING);
public static final Uri PART_CONTENT_URI = Uri.parse(PART_URI_STRING);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is intentionally private

@McLoo McLoo mentioned this pull request Jun 23, 2015
@McLoo
Copy link
Contributor Author

McLoo commented Jun 23, 2015

@moxie0 comments addressed

@McLoo
Copy link
Contributor Author

McLoo commented Jun 25, 2015

rebased

@McLoo
Copy link
Contributor Author

McLoo commented Jul 30, 2015

rebased. reduced refactoring.

@McLoo
Copy link
Contributor Author

McLoo commented Jul 31, 2015

馃槧 temporarily closing til I'm happy with GIF-detection

getting the MimeType isn't very funny for internal media :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants