scheme
ClosedPublic
Actions

Authored by angelika on Thu, Apr 10, 5:50 PM.

Details

Reviewers

bartek

Commits

rCOMM7be9e166f04c: [native] Fix uploading images with ph-upload:// scheme

Summary

https://linear.app/comm/issue/ENG-10531/unable-to-send-screenshots-in-latest-build

Related: D14512

Some images and videos on iOS have ph-upload:// scheme and they fail to be encrypted thus crashing the app. They work ok for processing though.
So we still use uri from image picker for processing but use local uri for encryption and upload.

Test Plan

Try to upload a couple of images on iOS: a screenshot, an image from a camera, a video from a camera etc.
Do the same for Android

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

angelika created this revision.Thu, Apr 10, 5:50 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptThu, Apr 10, 5:50 PM

Harbormaster completed remote builds in B33963: Diff 47748.Thu, Apr 10, 6:10 PM

angelika requested review of this revision.Thu, Apr 10, 6:10 PM

bartek accepted this revision.Thu, Apr 10, 11:41 PM

This revision is now accepted and ready to land.Thu, Apr 10, 11:41 PM

Closed by commit rCOMM7be9e166f04c: [native] Fix uploading images with ph-upload:// scheme (authored by angelika). · Explain WhyFri, Apr 11, 7:44 AM

This revision was automatically updated to reflect the committed changes.

angelika added a commit: rCOMM7be9e166f04c: [native] Fix uploading images with ph-upload:// scheme.

It seems like we're discarding the processed media, since we're returning uploadURI to the caller (non-processed media). I hope I'm missing something here, as both @angelika and @bartek did not flag this...

angelika mentioned this in D14574: [native] Fix uploading transcoded media.Fri, Apr 11, 10:12 AM

In D14566#404086, @ashoat wrote:

It seems like we're discarding the processed media, since we're returning uploadURI to the caller (non-processed media). I hope I'm missing something here, as both @angelika and @bartek did not flag this...

https://phab.comm.dev/D14574

That's right. There was a mistake here. During the review, I was comparing changes in this diff with changes from D14512 and I misunderstood changes to initialURI in that diff, making me think this diff is correct.

angelika mentioned this in rCOMMfbcddf5abbd0: [native] Fix uploading transcoded media.Mon, Apr 14, 7:40 AM

ashoat mentioned this in D14586: [native] Fix unprocessed media being uploaded.Sat, Apr 19, 10:18 PM

ashoat mentioned this in rCOMM6830e7e6ffab: [native] Fix unprocessed media being uploaded.Sat, Apr 19, 10:40 PM