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(ffi): Generate correct temporary file names for media attachments #1851

Merged
merged 2 commits into from
May 3, 2023

Conversation

stefanceriu
Copy link
Member

Fixes element-hq/element-x-ios#843

As the issue above describes, simple text files (and surely others) aren't currently renderable iOS side. This happens because we use mime_guess to infer an extension from the content type and that results in a list of extensions sorted in no particular order. In this case text/plain results in asm because it just happens to be the first match from this list which iOS doesn't know what to do with.

To fix that this PR switches over to mime2ext (like this comment from mime_guess suggests) which does return more commonly used extensions.

Separately, the body of an attachment is usually its original filename which we are currently not using. This PR also addresses that by adding an optional body property and trying to use it as much as possible when generating the temporary file name. This will in turn show up in the iOS system dialogs when trying to save said file.

@stefanceriu stefanceriu requested review from pixlwave and a team April 30, 2023 09:19
@codecov
Copy link

codecov bot commented Apr 30, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (034aa04) 76.44% compared to head (dfc20ed) 76.42%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1851      +/-   ##
==========================================
- Coverage   76.44%   76.42%   -0.02%     
==========================================
  Files         140      140              
  Lines       15977    15977              
==========================================
- Hits        12214    12211       -3     
- Misses       3763     3766       +3     
Impacted Files Coverage Δ
crates/matrix-sdk/src/media.rs 58.13% <ø> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@stefanceriu stefanceriu force-pushed the stefan/mediaFileExtensionGuessFix branch from d4f2149 to 906a573 Compare May 2, 2023 06:07
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Ok seems reasonable to me. Having thought about it, given the spec says the body should be the filename, it does seems reasonable to take that first and if it fails use the mimetype as a fallback. Let's see how it works in the real world 🤞

crates/matrix-sdk/src/media.rs Outdated Show resolved Hide resolved
@stefanceriu stefanceriu requested a review from jplatte May 2, 2023 12:02
crates/matrix-sdk/src/media.rs Outdated Show resolved Hide resolved
crates/matrix-sdk/src/media.rs Outdated Show resolved Hide resolved
crates/matrix-sdk/src/media.rs Show resolved Hide resolved
@stefanceriu stefanceriu force-pushed the stefan/mediaFileExtensionGuessFix branch from d0863c8 to 25a07f0 Compare May 2, 2023 15:35
@stefanceriu stefanceriu requested a review from jplatte May 2, 2023 15:36
@stefanceriu stefanceriu force-pushed the stefan/mediaFileExtensionGuessFix branch from 25a07f0 to dfc20ed Compare May 2, 2023 17:18
@stefanceriu stefanceriu requested a review from jplatte May 2, 2023 17:18
@jplatte jplatte merged commit 45cb5f0 into main May 3, 2023
@jplatte jplatte deleted the stefan/mediaFileExtensionGuessFix branch May 3, 2023 11:00
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.

.txt files aren't visible
3 participants