-
-
Notifications
You must be signed in to change notification settings - Fork 600
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
Handle relations in encrypted rooms #969
Conversation
src/client.js
Outdated
@@ -3991,14 +3993,25 @@ MatrixClient.prototype.getCanResetTimelineCallback = function() { | |||
*/ | |||
MatrixClient.prototype.relations = | |||
async function(roomId, eventId, relationType, eventType, opts = {}) { | |||
const isEncrypted = this.isRoomEncrypted(roomId); | |||
const fetchedEventType = isEncrypted ? "m.room.encrypted" : eventType; |
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.
Hmm, so this fetchedEventType
will be wrong for reactions, which are now always m.reaction
even in E2E rooms. 😓 It does seem nice to handle encryption transparently though... Maybe we should add a helper function similar to _encryptEventIfNeeded
called something like _getEncryptedIfNeededEventType
that returns the eventual event type (with a similar exception for reactions)?
let events = result.chunk.map(this.getEventMapper()); | ||
if (isEncrypted) { | ||
await Promise.all(events.map(e => { | ||
return new Promise(resolve => e.once("Event.decrypted", resolve)); |
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.
Reactions in an encrypted room won't be encrypted events themselves, so we can't use isEncrypted
for the whole room to decide whether to await Event.decrypted
.
Since the event mapper will invoke attemptDecryption
, I think you only want to wait for Event.decrypted
if event.isBeingDecrypted()
.
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, this looks good! 😁 Sorry reactions are creating trouble... 😅
Part of matrix-org/matrix-react-sdk#3151