feat(widget): Support for avatars in calls via msc4039#6354
feat(widget): Support for avatars in calls via msc4039#6354stefanceriu merged 10 commits intomainfrom
Conversation
Merging this PR will improve performance by 48.26%
Performance Changes
Comparing 
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6354 +/- ##
==========================================
+ Coverage 89.85% 89.86% +0.01%
==========================================
Files 378 378
Lines 103384 103491 +107
Branches 103384 103491 +107
==========================================
+ Hits 92893 93001 +108
+ Misses 6928 6924 -4
- Partials 3563 3566 +3 ☔ View full report in Codecov by Sentry. |
| // Not used yet. Only support non-encrypted downloads (avatars) | ||
| // pub(crate) timeout_ms: Option<u64>, | ||
| // pub(crate) encryption: Option<EncryptedFile>, |
There was a problem hiding this comment.
I would delete these for now. What even is an EncryptedFile in this context?
There was a problem hiding this comment.
Removed, details for that are in the MSC
| pub(crate) struct DownloadFileResponse { | ||
| // The binary file content in a format that can cross the | ||
| // widget-driver-api boundary. | ||
| pub(crate) file: Base64, |
There was a problem hiding this comment.
| pub(crate) file: Base64, | |
| pub(crate) file_data: Base64, |
There was a problem hiding this comment.
Ok, but then I have to use a #[serde(rename = "file")] to respect the spec.
If going that way, probably should call it file_data_base64
| match matrix_driver_response { | ||
| MatrixDriverResponse::FileDownloaded(resp) => Some(Self { file: resp.file }), | ||
| _ => { | ||
| error!("bug in MatrixDriver, received wrong event response"); |
There was a problem hiding this comment.
I wonder if this should be a full on panic as it's a developer error.
There was a problem hiding this comment.
Right, can this be a separate PR? because it is now the common pattern in the crate
| assert_eq!(req.content_uri, "mxc://server/id"); | ||
| } | ||
| #[test] | ||
| fn parse_download_file_widget_action_error() { |
There was a problem hiding this comment.
Took me a while to realised what's different here. Let's perhaps call it parse_uri_validation_error or something
There was a problem hiding this comment.
rename to parse_download_file_request_with_non_mxc_url
| } | ||
| FromWidgetRequest::DownloadFile(req) => self | ||
| .process_download_file_request(req, raw_request) | ||
| .map(|a| vec![a]) |
There was a problem hiding this comment.
Does anything in here actually return a list of actions or is always jsut the one?
There was a problem hiding this comment.
maybe some other widget requests returns several? I don't know
| &mut self, | ||
| req: DownloadFileRequest, | ||
| raw_request: Raw<FromWidgetRequest>, | ||
| ) -> Option<Action> { |
There was a problem hiding this comment.
This shouldn't return an optional.
There was a problem hiding this comment.
This is the consistent pattern used in this file for all action handling. Maybe this can be another PR to discuss and change this pattern
| to true will now trigger a download of all historical keys for the room in | ||
| question from the client's key backup. | ||
| ([#6017](https://github.com/matrix-org/matrix-rust-sdk/pull/6017)) | ||
| - Add widget support for MSC4039. Allows widgets to download files from the |
There was a problem hiding this comment.
Perhaps we should mention it only works for non-encrypted files.
| } | ||
|
|
||
| #[async_test] | ||
| async fn test_try_download_external() { |
There was a problem hiding this comment.
Same comment as above I guess 🤷♂️
There was a problem hiding this comment.
test_download_non_mxc_uri_should_fail ?
2 participants
Allows to get the avatars in Element Call widget !
Using MSC4039
Counter part of web support:
Needs a EC PR to support passing data as base64 via the widget json API
msc4039 support b64 in addition to blob file download element-hq/element-call#3818
I've documented the public API Changes in the appropriate
CHANGELOG.mdfiles.This PR was made with the help of AI.
Signed-off-by: