fix: Use getFilename() instead of the actual filename on disk (#3521)

* fix: Use msg.getFilename() instead of the file's name in some cases

* fix: Use msg.getFilename() instead of the file's name in initializeDraft()

* fix: Use msg.getFilename() instead of the file's name in MediaItem

* fix: Use the correct file name in MediaView

* refactor: `msg` param of `getManuallyCalculatedSlideInfo()` was always null

* Improve comment

* Revert "refactor: `msg` param of `getManuallyCalculatedSlideInfo()` was always null"

We will unfortunately need getManuallyCalculatedSlideInfo() with `msg`
param

This reverts commit 60e8248db3.

* fix: Fix drafting images

This fixes a bug introduced in 14f69f87e8:
When you drafted an image, pressed Back, and opened the chat again, then
the height of the drafted image was wrong and tapping the image opened a
preview for the wrong image.

I do think that theoretically it would be nicer to use getSlideForMsg
here, because we already have a DcMsg, but this didn't work because a)
the width and height wasn't gotten from the msg and instead 0 was passed
and b) the code tries to save a msgId instead of the message instead,
and loading the message from the database fails later since it's just a
draft.

I didn't want to try and fix these things, because they might be bigger
refactorings and I don't know the code.

* fix: Use the original message's filemime if there is one

...instead of trying to guess the mimetype from the uri
This commit is contained in:
Hocuri 2025-01-15 16:40:23 +01:00 committed by GitHub
parent 923227a0e8
commit 8e55a3dbf3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 60 additions and 46 deletions

View file

@ -93,7 +93,7 @@ public class SharingTest {
}
}
Uri uri = Uri.parse("content://" + BuildConfig.APPLICATION_ID + ".attachments/" + Uri.encode(pngImage));
DcHelper.sharedFiles.put("/" + pngImage, 1);
DcHelper.sharedFiles.put(pngImage, 1);
Intent i = new Intent(Intent.ACTION_SEND);
i.setType("image/png");

View file

@ -120,7 +120,6 @@ import org.thoughtcrime.securesms.util.ViewUtil;
import org.thoughtcrime.securesms.util.concurrent.AssertedSuccessListener;
import org.thoughtcrime.securesms.util.guava.Optional;
import org.thoughtcrime.securesms.util.views.ProgressDialog;
import org.thoughtcrime.securesms.util.views.Stub;
import org.thoughtcrime.securesms.video.recode.VideoRecoder;
import org.thoughtcrime.securesms.videochat.VideochatUtil;
@ -746,8 +745,8 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
handleReplyMessage(quote);
}
String filename = draft.getFile();
if (filename.isEmpty() || !new File(filename).exists()) {
String file = draft.getFile();
if (file.isEmpty() || !new File(file).exists()) {
future.set(!text.isEmpty());
updateToggleButtonState();
return future;
@ -767,26 +766,21 @@ public class ConversationActivity extends PassphraseRequiredActionBarActivity
}
};
File file = new File(filename);
Uri uri = Uri.fromFile(file);
switch (draft.getType()) {
case DcMsg.DC_MSG_IMAGE:
setMedia(uri, MediaType.IMAGE).addListener(listener);
setMedia(draft, MediaType.IMAGE).addListener(listener);
break;
case DcMsg.DC_MSG_GIF:
setMedia(uri, MediaType.GIF).addListener(listener);
setMedia(draft, MediaType.GIF).addListener(listener);
break;
case DcMsg.DC_MSG_AUDIO:
setMedia(uri, MediaType.AUDIO).addListener(listener);
setMedia(draft, MediaType.AUDIO).addListener(listener);
break;
case DcMsg.DC_MSG_VIDEO:
setMedia(uri, MediaType.VIDEO).addListener(listener);
break;
case DcMsg.DC_MSG_WEBXDC:
setMedia(draft, MediaType.DOCUMENT).addListener(listener);
setMedia(draft, MediaType.VIDEO).addListener(listener);
break;
default:
setMedia(uri, MediaType.DOCUMENT).addListener(listener);
setMedia(draft, MediaType.DOCUMENT).addListener(listener);
break;
}

View file

@ -207,7 +207,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
messageRecord = null;
long date = getIntent().getLongExtra(DATE_EXTRA, 0);
long size = getIntent().getLongExtra(SIZE_EXTRA, 0);
initialMedia = new MediaItem(null, getIntent().getData(), getIntent().getType(),
initialMedia = new MediaItem(null, getIntent().getData(), null, getIntent().getType(),
DcMsg.DC_MSG_NO_ID, date, size, false);
if (address != null) {
@ -218,7 +218,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
} else {
messageRecord = dcContext.getMsg(msgId);
initialMedia = new MediaItem(Recipient.fromChat(context, msgId), Uri.fromFile(messageRecord.getFileAsFile()),
messageRecord.getFilemime(), messageRecord.getId(), messageRecord.getDateReceived(),
messageRecord.getFilename(), messageRecord.getFilemime(), messageRecord.getId(), messageRecord.getDateReceived(),
messageRecord.getFilebytes(), messageRecord.isOutgoing());
conversationRecipient = Recipient.fromChat(context, msgId);
}
@ -232,12 +232,11 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
// if you search for the place where the media are loaded, go to 'onCreateLoader'.
Log.w(TAG, "Loading Part URI: " + initialMedia);
if (messageRecord != null) {
getSupportLoaderManager().restartLoader(0, null, this);
} else {
mediaPager.setAdapter(new SingleItemPagerAdapter(this, GlideApp.with(this),
getWindow(), initialMedia.uri, initialMedia.type, initialMedia.size));
getWindow(), initialMedia.uri, initialMedia.name, initialMedia.type, initialMedia.size));
}
}
@ -313,7 +312,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
private void performSavetoDisk(@NonNull MediaItem mediaItem) {
SaveAttachmentTask saveTask = new SaveAttachmentTask(MediaPreviewActivity.this);
long saveDate = (mediaItem.date > 0) ? mediaItem.date : System.currentTimeMillis();
saveTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, new Attachment(mediaItem.uri, mediaItem.type, saveDate, null));
saveTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, new Attachment(mediaItem.uri, mediaItem.type, saveDate, mediaItem.name));
}
private void showInChat() {
@ -486,18 +485,20 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
private final GlideRequests glideRequests;
private final Window window;
private final Uri uri;
private final String name;
private final String mediaType;
private final long size;
private final LayoutInflater inflater;
SingleItemPagerAdapter(@NonNull Context context, @NonNull GlideRequests glideRequests,
@NonNull Window window, @NonNull Uri uri, @NonNull String mediaType,
@NonNull Window window, @NonNull Uri uri, @Nullable String name, @NonNull String mediaType,
long size)
{
this.glideRequests = glideRequests;
this.window = window;
this.uri = uri;
this.name = name;
this.mediaType = mediaType;
this.size = size;
this.inflater = LayoutInflater.from(context);
@ -519,7 +520,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
MediaView mediaView = itemView.findViewById(R.id.media_view);
try {
mediaView.set(glideRequests, window, uri, mediaType, size, true);
mediaView.set(glideRequests, window, uri, name, mediaType, size, true);
} catch (IOException e) {
Log.w(TAG, e);
}
@ -539,7 +540,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
@Override
public MediaItem getMediaItemFor(int position) {
return new MediaItem(null, uri, mediaType, DcMsg.DC_MSG_NO_ID, -1, -1, true);
return new MediaItem(null, uri, name, mediaType, DcMsg.DC_MSG_NO_ID, -1, -1, true);
}
@Override
@ -604,7 +605,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
try {
//noinspection ConstantConditions
mediaView.set(glideRequests, window, Uri.fromFile(msg.getFileAsFile()),
mediaView.set(glideRequests, window, Uri.fromFile(msg.getFileAsFile()), msg.getFilename(),
msg.getFilemime(), msg.getFilebytes(), autoplay);
} catch (IOException e) {
Log.w(TAG, e);
@ -633,6 +634,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
return new MediaItem(Recipient.fromChat(context, msg.getId()),
Uri.fromFile(msg.getFileAsFile()),
msg.getFilename(),
msg.getFilemime(),
msg.getId(),
msg.getDateReceived(),
@ -655,6 +657,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
private static class MediaItem {
private final @Nullable Recipient recipient;
private final @NonNull Uri uri;
private final @Nullable String name;
private final @NonNull String type;
private final int msgId;
private final long date;
@ -663,6 +666,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
private MediaItem(@Nullable Recipient recipient,
@NonNull Uri uri,
@Nullable String name,
@NonNull String type,
int msgId,
long date,
@ -671,6 +675,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity
{
this.recipient = recipient;
this.uri = uri;
this.name = name;
this.type = type;
this.msgId = msgId;
this.date = date;

View file

@ -3,10 +3,10 @@ package org.thoughtcrime.securesms.components;
import android.content.Context;
import android.net.Uri;
import android.os.Build;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.RequiresApi;
import android.util.AttributeSet;
import android.view.View;
import android.view.Window;
@ -55,6 +55,7 @@ public class MediaView extends FrameLayout {
public void set(@NonNull GlideRequests glideRequests,
@NonNull Window window,
@NonNull Uri source,
@Nullable String fileName,
@NonNull String mediaType,
long size,
boolean autoplay)
@ -68,7 +69,7 @@ public class MediaView extends FrameLayout {
imageView.setVisibility(View.GONE);
videoView.get().setVisibility(View.VISIBLE);
videoView.get().setWindow(window);
videoView.get().setVideoSource(new VideoSlide(getContext(), source, size), autoplay);
videoView.get().setVideoSource(new VideoSlide(getContext(), source, fileName, size), autoplay);
} else {
throw new IOException("Unsupported media type: " + mediaType);
}

View file

@ -29,7 +29,12 @@ public class AttachmentsContentProvider extends ContentProvider {
public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException {
DcContext dcContext = DcHelper.getContext(getContext());
String path = uri.getPath();
// `uri` originally comes from DcHelper.openForViewOrShare() and
// looks like `content://chat.delta.attachments/ef39a39/text.txt`
// where ef39a39 is the file in the blob directory
// and text.txt is the original name of the file, as returned by `msg.getFilename()`.
// `uri.getPathSegments()` returns ["ef39a39", "text.txt"] in this example.
String path = uri.getPathSegments().get(0);
if (!DcHelper.sharedFiles.containsKey(path)) {
throw new FileNotFoundException("File was not shared before.");
}

View file

@ -281,6 +281,7 @@ public class DcHelper {
DcMsg msg = dcContext.getMsg(msg_id);
String path = msg.getFile();
String filename = msg.getFilename();
String mimeType = msg.getFilemime();
try {
File file = new File(path);
@ -291,8 +292,11 @@ public class DcHelper {
Uri uri;
if (path.startsWith(dcContext.getBlobdir())) {
uri = Uri.parse("content://" + BuildConfig.APPLICATION_ID + ".attachments/" + Uri.encode(file.getName()));
sharedFiles.put("/" + file.getName(), 1); // as different Android version handle uris in putExtra differently, we also check them on our own
// Build a Uri that will later be passed to AttachmentsContentProvider.openFile().
// The last part needs to be `filename`, i.e. the original, user-visible name of the file,
// so that the external apps show the name of the file correctly.
uri = Uri.parse("content://" + BuildConfig.APPLICATION_ID + ".attachments/" + Uri.encode(file.getName()) + "/" + Uri.encode(filename));
sharedFiles.put(file.getName(), 1); // as different Android version handle uris in putExtra differently, we also check them on our own
} else {
if (Build.VERSION.SDK_INT >= 24) {
uri = FileProvider.getUriForFile(activity, BuildConfig.APPLICATION_ID + ".fileprovider", file);
@ -302,7 +306,7 @@ public class DcHelper {
}
if (cmd.equals(Intent.ACTION_VIEW)) {
mimeType = checkMime(path, mimeType);
mimeType = checkMime(filename, mimeType);
Intent intent = new Intent(Intent.ACTION_VIEW);
intent.setDataAndType(uri, mimeType);
intent.setFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);

View file

@ -257,11 +257,11 @@ public class AttachmentManager {
return new DocumentSlide(context, msg);
}
else if (PartAuthority.isLocalUri(uri)) {
return getManuallyCalculatedSlideInfo(uri, width, height);
return getManuallyCalculatedSlideInfo(uri, width, height, msg);
} else {
Slide result = getContentResolverSlideInfo(uri, width, height, chatId);
if (result == null) return getManuallyCalculatedSlideInfo(uri, width, height);
if (result == null) return getManuallyCalculatedSlideInfo(uri, width, height, msg);
else return result;
}
} catch (IOException e) {
@ -362,16 +362,21 @@ public class AttachmentManager {
return null;
}
private @NonNull Slide getManuallyCalculatedSlideInfo(Uri uri, int width, int height) throws IOException {
private @NonNull Slide getManuallyCalculatedSlideInfo(Uri uri, int width, int height, @Nullable DcMsg msg) throws IOException {
long start = System.currentTimeMillis();
Long mediaSize = null;
String fileName = null;
String mimeType = null;
if (msg != null) {
fileName = msg.getFilename();
mimeType = msg.getFilemime();
}
if (PartAuthority.isLocalUri(uri)) {
mediaSize = PartAuthority.getAttachmentSize(context, uri);
fileName = PartAuthority.getAttachmentFileName(context, uri);
mimeType = PartAuthority.getAttachmentContentType(context, uri);
if (fileName == null) fileName = PartAuthority.getAttachmentFileName(context, uri);
if (mimeType == null) mimeType = PartAuthority.getAttachmentContentType(context, uri);
}
if (mediaSize == null) {
@ -676,10 +681,10 @@ public class AttachmentManager {
}
switch (this) {
case IMAGE: return new ImageSlide(context, uri, dataSize, width, height);
case GIF: return new GifSlide(context, uri, dataSize, width, height);
case IMAGE: return new ImageSlide(context, uri, fileName, dataSize, width, height);
case GIF: return new GifSlide(context, uri, fileName, dataSize, width, height);
case AUDIO: return new AudioSlide(context, uri, dataSize, false, fileName);
case VIDEO: return new VideoSlide(context, uri, dataSize);
case VIDEO: return new VideoSlide(context, uri, fileName, dataSize);
case DOCUMENT:
// We have to special-case Webxdc slides: The user can interact with them as soon as a draft
// is set. Therefore we need to create a DcMsg already now.

View file

@ -14,8 +14,8 @@ public class GifSlide extends ImageSlide {
super(context, dcMsg);
}
public GifSlide(Context context, Uri uri, long size, int width, int height) {
super(context, constructAttachmentFromUri(context, uri, MediaUtil.IMAGE_GIF, size, width, height, uri, null, false));
public GifSlide(Context context, Uri uri, String fileName, long size, int width, int height) {
super(context, constructAttachmentFromUri(context, uri, MediaUtil.IMAGE_GIF, size, width, height, uri, fileName, false));
}
@Override

View file

@ -39,8 +39,8 @@ public class ImageSlide extends Slide {
super(context, attachment);
}
public ImageSlide(Context context, Uri uri, long size, int width, int height) {
super(context, constructAttachmentFromUri(context, uri, MediaUtil.IMAGE_JPEG, size, width, height, uri, null, false));
public ImageSlide(Context context, Uri uri, String fileName, long size, int width, int height) {
super(context, constructAttachmentFromUri(context, uri, MediaUtil.IMAGE_JPEG, size, width, height, uri, fileName, false));
}
@Override

View file

@ -30,16 +30,16 @@ import java.io.File;
public class VideoSlide extends Slide {
private static Attachment constructVideoAttachment(Context context, Uri uri, long dataSize)
private static Attachment constructVideoAttachment(Context context, Uri uri, String fileName, long dataSize)
{
Uri thumbnailUri = Uri.fromFile(new File(DcHelper.getBlobdirFile(DcHelper.getContext(context), "temp-preview.jpg")));
MediaUtil.ThumbnailSize retWh = new MediaUtil.ThumbnailSize(0, 0);
MediaUtil.createVideoThumbnailIfNeeded(context, uri, thumbnailUri, retWh);
return constructAttachmentFromUri(context, uri, MediaUtil.VIDEO_UNSPECIFIED, dataSize, retWh.width, retWh.height, thumbnailUri, null, false);
return constructAttachmentFromUri(context, uri, MediaUtil.VIDEO_UNSPECIFIED, dataSize, retWh.width, retWh.height, thumbnailUri, fileName, false);
}
public VideoSlide(Context context, Uri uri, long dataSize) {
super(context, constructVideoAttachment(context, uri, dataSize));
public VideoSlide(Context context, Uri uri, String fileName, long dataSize) {
super(context, constructVideoAttachment(context, uri, fileName, dataSize));
}
public VideoSlide(Context context, DcMsg dcMsg) {