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.

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

Fix not hiding redacted edits #978

Closed
wants to merge 2 commits into from

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Jul 4, 2019

Fixes https://github.com/vector-im/riot-web/issues/10112

Preserves rel_type after redacting an edit, so we still know it wasn't a normal message

@bwindels bwindels requested a review from a team July 4, 2019 15:56
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

otherwise lgtm

// 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

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...

@bwindels
Copy link
Contributor Author

bwindels commented Jul 5, 2019

This needs backend support to solve properly, see matrix-org/matrix-spec-proposals#2154 (comment)

@bwindels bwindels closed this Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants