-
Notifications
You must be signed in to change notification settings - Fork 379
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
MSC4039: Access the Content repository with the Widget API #4039
base: main
Are you sure you want to change the base?
MSC4039: Access the Content repository with the Widget API #4039
Conversation
… repository Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the nice write-up, I think this is very reasonable as an extension to the widget API as it currently exists (and it's an extension I was already expecting to be made sooner or later)
Signed-off-by: Dominik Henneke <dominik.henneke@nordeck.net>
Co-authored-by: Johannes Marbach <n0-0ne+github@mailbox.org>
- added download API to handle MSC3916 "Authentication for media" - added e2ee aware variants for both upload and download Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
Signed-off-by: Kim Brose <kim.brose@nordeck.net>
extended such that whenever a widget includes references to files in an event, it is required to provide them to the | ||
client in the `send_event` request so the client knows to attach the files. | ||
|
||
## Alternatives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could of course also pass the client's access token to the widget by URI param or with a request, but that seems to contradict the whole idea of the widget API
In line with matrix-org/matrix-spec#1700, the following disclosure applies: I am an active member of the Matrix FOSS community, member of the Matrix Foundation governing board, and employed by Nordeck. My parts of this proposal are written and published as a Nordeck employee. (Similar applies to the other commits to this PR as indicated by the origin branch coming from the |
Implementations:
|
} | ||
``` | ||
|
||
`response.file` mirrors `data.file` of the `upload_file` action and is a `XMLHttpRequestBodyInit` that is supported as a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`response.file` mirrors `data.file` of the `upload_file` action and is a `XMLHttpRequestBodyInit` that is supported as a | |
`response.file` mirrors `data.file` of the `upload_file` action and is an `XMLHttpRequestBodyInit` that is supported as a |
} | ||
``` | ||
|
||
`data.file` is a `XMLHttpRequestBodyInit` that is supported as a data type in the `postMessage` API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`data.file` is a `XMLHttpRequestBodyInit` that is supported as a data type in the `postMessage` API. | |
`data.file` is an `XMLHttpRequestBodyInit` that is supported as a data type in the `postMessage` API. |
} | ||
``` | ||
|
||
`data.file` is a `XMLHttpRequestBodyInit` that is supported as a data type in the `postMessage` API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like quite a random type? Why not just blob or something? Do we explicitly want the API to support passing, say, FormData through directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not easy to answer, since the team members originally proposing that type have left the team. We did not change the type as we updated the MSC because the upload part is already implemented. The reason seems to be "Something that can be sent to XMLHttpRequest.send (typically a File)." https://github.com/matrix-org/matrix-widget-api/pull/86/files#diff-67e6f0676307a87283ca4533526de9b3ac4ff3bafdd0d016947e40e2b3574757R693-R694. However we can follow your reasoning that probably a Blob (which is one of the types forming this union type) would make more sense.
"requestid": "generated-id-1234", | ||
"action": "upload_file", | ||
"data": { | ||
"file": "some-content" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'file' here sort of implies that this is a https://developer.mozilla.org/en-US/docs/Web/API/File which it is not. This sort of goes with my other comment below on what type this is and what it's called.
} | ||
``` | ||
|
||
`response.file` mirrors `data.file` of the `upload_file` action and is a `XMLHttpRequestBodyInit` that is supported as a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above for 'file', plus it will be the widget's job to support whatever the client decides to send it of the union of different types, which might be an easy trap for widget authors to fall into. The impl in the react-sdk PR just always sends a blob anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we decide to change the type to Blob, perhaps calling both fields blob would make the most sense.
the API described above to add the `copy` endpoint. Additionally, the event sending payload from MSC2762 will need to be | ||
extended such that whenever a widget includes references to files in an event, it is required to provide them to the | ||
client in the `send_event` request so the client knows to attach the files. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, large files will have to be sent over the postmessage API in one big lump, so this isn't going to scale up to anything over a few tens of MB very well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One alternative already described could be letting widgets directly access (parts of) the C-S API in some way.
Rendered
Implementations