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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented Jan 6, 2025

In preparation for deltachat/deltachat-core-rust#6332, use the original name of the file (as returned by getFilename()) instead of the current filename on disk.

Most of the commits are cherry-picked from #3513.

What I did:

  • Grep for usages of DcMsg.getFile(), check where it's passed, and make sure that all it's never used to check the file extension, to pass it to an external application, or to show it to the user.
  • Look at places in the code where a filename is user-visible, like when viewing it in the UI or passing it to an external application (especially when sharing or opening it). Check where the filename comes from, and make sure that it comes from getFilename(), not from getName().
  • Test that 1. sharing to and from DC 2. opening 3. saving ("Export attachment") 4. drafting and re-entering the chat 5. sending still works for a) Images b) Videos c) vCards (attached contacts) d) attached files e) webxdc's. Also that editing images works. (I hope I didn't forget to test any combination)

Known bugs:

  • The height isn't correctly for image and video drafts calculated because getManuallyCalculatedSlideInfo() isn't called, so that DcAttachment gets width and height 0
  • When tapping the preview of a draft image, a wrong image is shown

@Hocuri Hocuri requested review from r10s and adbenitez January 6, 2025 18:50

This comment was marked as outdated.

@Hocuri Hocuri changed the title fix: Use getFilename() instead of the actual filename on disk [WIP] fix: Use getFilename() instead of the actual filename on disk Jan 6, 2025
@Hocuri

This comment was marked as outdated.

Hocuri added 2 commits January 7, 2025 14:36
…as always null"

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

This reverts commit 60e8248.
This fixes a bug introduced in 14f69f8:
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.
@Hocuri Hocuri changed the title [WIP] fix: Use getFilename() instead of the actual filename on disk fix: Use getFilename() instead of the actual filename on disk Jan 7, 2025
@Hocuri

This comment was marked as outdated.

DcHelper.sharedFiles.put("/" + pngImage, 1);
DcHelper.sharedFiles.put(pngImage, 1);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For explanation: Since AttachmentsContentProvider.openFile() (which checks sharedFiles) now uses getPathSegments().get(0); instead of getPath();, the leading / won't be included there. So, it's easier for AttachmentsContentProvider.openFile() to have it without the leading /.

Copy link

github-actions bot commented Jan 7, 2025

To test the changes in this pull request, install this apk:
📦 app-preview.apk

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

Successfully merging this pull request may close these issues.

1 participant