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.

Sign up for GitHub

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

Jump to bottom

Fix not hiding redacted edits #978

Closed
wants to merge 2 commits into from
Closed

Fix not hiding redacted edits #978

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
955fddf
preserver rel_type after redacting an event
bwindels Jul 4, 2019
57084ca
add comment why we need this
bwindels Jul 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/models/event.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ module.exports.MatrixEvent = function MatrixEvent(
this._pushActions = null;
this._replacingEvent = null;
this._localRedactionEvent = null;
this._relationTypeBeforeRedaction = null;
this._isCancelled = false;

this._clearEvent = {};
Expand Down Expand Up @@ -712,6 +713,13 @@ utils.extend(module.exports.MatrixEvent.prototype, {

this.emit("Event.beforeRedaction", this, redaction_event);

// before removing content, preserve the rel_type
// if this is a relation, so we can still tell the
// difference between a redacted edit and message
const relation = this.getRelation();
if (relation) {
this._relationTypeBeforeRedaction = relation.rel_type;
Copy link
Member

Choose a reason for hiding this comment

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

is this field persisted? Concern would be it showing correctly at the time it happened, but when switching rooms or refreshing the page the double redaction comes back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think it needs to be. All events are kept in memory, so switching rooms shouldn't clear this.
As for refreshing the page, both the edit and its redaction should be loaded in the same order when processing the sync accumulator blob, so that should also work.

Copy link
Contributor Author

@bwindels bwindels Jul 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Argh, you might be right. Just saw them popup again (fetched through /messages I presume). I was assuming redacted messages wouldn't show up through /messages, but looks like that is not the case... will discuss in the reactions channel...

}
this._replacingEvent = null;
// we attempt to replicate what we would see from the server if
// the event had been redacted before we saw it.
Expand Down Expand Up @@ -843,12 +851,20 @@ utils.extend(module.exports.MatrixEvent.prototype, {
* @return {boolean}
*/
isRelation(relType = undefined) {
let eventRelType;
// Relation info is lifted out of the encrypted content when sent to
// encrypted rooms, so we have to check `getWireContent` for this.
const content = this.getWireContent();
const relation = content && content["m.relates_to"];
return relation && relation.rel_type && relation.event_id &&
((relType && relation.rel_type === relType) || !relType);
if (relation) {
if (!relation || !relation.event_id) {
return false;
}
eventRelType = relation && relation.rel_type;
} else if (this.isRedacted()) {
eventRelType = this._relationTypeBeforeRedaction;
}
return eventRelType && ((relType && eventRelType === relType) || !relType);
},

/**
Expand Down