Support rendering some media downloads as inline #1360
Conversation
| @@ -45,7 +45,7 @@ | |||
| =head2 upload_test_content | |||
|
|
|||
| my ( $content_id, $content_uri ) = upload_test_content( | |||
| $user, filename => "filename", | |||
| $user, filename => "filename", $content_type | |||
There was a problem hiding this comment.
Not a fan of mixing positionals and named arguments like that.
We could make it another param, and skip it when using %params in its entirety, like this:
my ($user, %params) = @_;
my $content_type = delete $params{content_type} //= "application/octet-stream";
This is using the defined-or construct introduced in Perl 5.10 – which is over 15 years old, though it is tradition that if the version is not declared anywhere, we should assume 5.8 featureset. I see a bunch of use 5.010; elsewhere in the codebase though, so we should be in the clear here.
There was a problem hiding this comment.
Can't modify delete in defined or assignment (//=) at tests/51media/01unicode.pl line 61, at EOF :(
There was a problem hiding this comment.
Oh yes, I got ahead of myself there. That should be just //, not //=. We already have the assignment in that line :)
tests/51media/01unicode.pl
Outdated
| @@ -125,7 +129,14 @@ sub parse_content_disposition_params { | |||
| # the first part must be 'attachment' | |||
| my $k = shift @parts; | |||
| my $v = shift @parts; | |||
| assert_eq( $k, "attachment", "content-disposition" ); | |||
|
|
|||
| if ($allow_inline_disposition eq 1) { | |||
There was a problem hiding this comment.
eq is for string comparison, which is works here but it needlessly explicit and out of place.
| if ($allow_inline_disposition eq 1) { | |
| if ($allow_inline_disposition) { |
| } | ||
| Future->done( $cd_params, $body ); | ||
| }); | ||
| } | ||
| push @EXPORT, qw( get_media ); | ||
|
|
||
| sub parse_content_disposition_params { | ||
| my ( $disposition ) = @_; | ||
| my ( $disposition, $allow_inline_disposition ) = @_; |
There was a problem hiding this comment.
I'd consider making this a named argument to make it more readable on the caller side (more than a plain 1 in the argument list).
|
matrix-org/complement#646 implements this test...so we can just drop it. |
|
The |
For matrix-org/synapse#15988
This is incredibly hamfisted, as my ability to write perl is limited. The idea here is that we allow either inline or attachment depositions for certain content-types.