diff --git a/src/Unread.ts b/src/Unread.ts index e596bdbe50e..7deafef9678 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -48,7 +48,7 @@ export function eventTriggersUnreadCount(ev: MatrixEvent): boolean { } if (ev.isRedacted()) return false; - return haveRendererForEvent(ev); + return haveRendererForEvent(ev, false /* hidden messages should never trigger unread counts anyways */); } export function doesRoomHaveUnreadMessages(room: Room): boolean { diff --git a/src/components/structures/MessagePanel.tsx b/src/components/structures/MessagePanel.tsx index a4791a1ec57..ff9bc826528 100644 --- a/src/components/structures/MessagePanel.tsx +++ b/src/components/structures/MessagePanel.tsx @@ -242,7 +242,7 @@ export default class MessagePanel extends React.Component { // displayed event in the current render cycle. private readReceiptsByUserId: Record = {}; - private readonly showHiddenEventsInTimeline: boolean; + private readonly _showHiddenEvents: boolean; private readonly threadsEnabled: boolean; private isMounted = false; @@ -270,7 +270,7 @@ export default class MessagePanel extends React.Component { // Cache these settings on mount since Settings is expensive to query, // and we check this in a hot code path. This is also cached in our // RoomContext, however we still need a fallback for roomless MessagePanels. - this.showHiddenEventsInTimeline = SettingsStore.getValue("showHiddenEventsInTimeline"); + this._showHiddenEvents = SettingsStore.getValue("showHiddenEventsInTimeline"); this.threadsEnabled = SettingsStore.getValue("feature_thread"); this.showTypingNotificationsWatcherRef = @@ -465,7 +465,7 @@ export default class MessagePanel extends React.Component { }; public get showHiddenEvents(): boolean { - return this.context?.showHiddenEventsInTimeline ?? this.showHiddenEventsInTimeline; + return this.context?.showHiddenEvents ?? this._showHiddenEvents; } // TODO: Implement granular (per-room) hide options @@ -748,7 +748,7 @@ export default class MessagePanel extends React.Component { const willWantDateSeparator = this.wantsDateSeparator(mxEv, nextEv.getDate() || new Date()); lastInSection = willWantDateSeparator || mxEv.getSender() !== nextEv.getSender() || - getEventDisplayInfo(nextEv).isInfoMessage || + getEventDisplayInfo(nextEv, this.showHiddenEvents).isInfoMessage || !shouldFormContinuation( mxEv, nextEv, this.showHiddenEvents, this.threadsEnabled, this.context.timelineRenderingType, ); diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 321c598ea6b..f53e75db0be 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -199,7 +199,7 @@ export interface IRoomState { showTwelveHourTimestamps: boolean; readMarkerInViewThresholdMs: number; readMarkerOutOfViewThresholdMs: number; - showHiddenEventsInTimeline: boolean; + showHiddenEvents: boolean; showReadReceipts: boolean; showRedactions: boolean; showJoinLeaves: boolean; @@ -271,7 +271,7 @@ export class RoomView extends React.Component { showTwelveHourTimestamps: SettingsStore.getValue("showTwelveHourTimestamps"), readMarkerInViewThresholdMs: SettingsStore.getValue("readMarkerInViewThresholdMs"), readMarkerOutOfViewThresholdMs: SettingsStore.getValue("readMarkerOutOfViewThresholdMs"), - showHiddenEventsInTimeline: SettingsStore.getValue("showHiddenEventsInTimeline"), + showHiddenEvents: SettingsStore.getValue("showHiddenEventsInTimeline"), showReadReceipts: true, showRedactions: true, showJoinLeaves: true, @@ -328,7 +328,7 @@ export class RoomView extends React.Component { this.setState({ readMarkerOutOfViewThresholdMs: value as number }), ), SettingsStore.watchSetting("showHiddenEventsInTimeline", null, (...[,,, value]) => - this.setState({ showHiddenEventsInTimeline: value as boolean }), + this.setState({ showHiddenEvents: value as boolean }), ), ]; } @@ -1480,7 +1480,7 @@ export class RoomView extends React.Component { continue; } - if (!haveRendererForEvent(mxEv, this.state.showHiddenEventsInTimeline)) { + if (!haveRendererForEvent(mxEv, this.state.showHiddenEvents)) { // XXX: can this ever happen? It will make the result count // not match the displayed count. continue; diff --git a/src/components/structures/ThreadPanel.tsx b/src/components/structures/ThreadPanel.tsx index 03798198adc..1dd37a8c412 100644 --- a/src/components/structures/ThreadPanel.tsx +++ b/src/components/structures/ThreadPanel.tsx @@ -250,7 +250,7 @@ const ThreadPanel: React.FC = ({ { const shouldIgnore = !!ev.status || // local echo (ignoreOwn && ev.getSender() === myUserId); // own message - const isWithoutTile = !haveRendererForEvent(ev, this.context?.showHiddenEventsInTimeline) || + const isWithoutTile = !haveRendererForEvent(ev, this.context?.showHiddenEvents) || shouldHideEvent(ev, this.context); if (isWithoutTile || !node) { diff --git a/src/components/views/messages/TextualEvent.tsx b/src/components/views/messages/TextualEvent.tsx index c92d7b55c7d..fb217607fef 100644 --- a/src/components/views/messages/TextualEvent.tsx +++ b/src/components/views/messages/TextualEvent.tsx @@ -28,7 +28,7 @@ export default class TextualEvent extends React.Component { static contextType = RoomContext; public render() { - const text = TextForEvent.textForEvent(this.props.mxEvent, true, this.context?.showHiddenEventsInTimeline); + const text = TextForEvent.textForEvent(this.props.mxEvent, true, this.context?.showHiddenEvents); if (!text) return null; return
{ text }
; } diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index f886482fbaf..722b77b5af0 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -1214,21 +1214,24 @@ export class UnwrappedEventTile extends React.Component { msgOption = readAvatars; } - const replyChain = - (haveRendererForEvent(this.props.mxEvent) && shouldDisplayReply(this.props.mxEvent)) - ? - : null; + let replyChain; + if ( + haveRendererForEvent(this.props.mxEvent, this.context.showHiddenEvents) && + shouldDisplayReply(this.props.mxEvent) + ) { + replyChain = ; + } const isOwnEvent = this.props.mxEvent?.sender?.userId === MatrixClientPeg.get().getUserId(); @@ -1267,7 +1270,7 @@ export class UnwrappedEventTile extends React.Component { highlightLink: this.props.highlightLink, onHeightChanged: this.props.onHeightChanged, permalinkCreator: this.props.permalinkCreator, - }) } + }, this.context.showHiddenEvents) } , ]); } @@ -1309,7 +1312,7 @@ export class UnwrappedEventTile extends React.Component { highlightLink: this.props.highlightLink, onHeightChanged: this.props.onHeightChanged, permalinkCreator: this.props.permalinkCreator, - }) } + }, this.context.showHiddenEvents) } { actionBar } { timestamp } @@ -1395,7 +1398,7 @@ export class UnwrappedEventTile extends React.Component { highlightLink: this.props.highlightLink, onHeightChanged: this.props.onHeightChanged, permalinkCreator: this.props.permalinkCreator, - }) } + }, this.context.showHiddenEvents) } , { highlightLink: this.props.highlightLink, onHeightChanged: this.props.onHeightChanged, permalinkCreator: this.props.permalinkCreator, - }) } + }, this.context.showHiddenEvents) } { keyRequestInfo } { actionBar } { this.props.layout === Layout.IRC && <> diff --git a/src/components/views/rooms/ReplyTile.tsx b/src/components/views/rooms/ReplyTile.tsx index bfd592f6985..2b973abfca5 100644 --- a/src/components/views/rooms/ReplyTile.tsx +++ b/src/components/views/rooms/ReplyTile.tsx @@ -110,7 +110,9 @@ export default class ReplyTile extends React.PureComponent { const msgType = mxEvent.getContent().msgtype; const evType = mxEvent.getType() as EventType; - const { hasRenderer, isInfoMessage, isSeeingThroughMessageHiddenForModeration } = getEventDisplayInfo(mxEvent); + const { + hasRenderer, isInfoMessage, isSeeingThroughMessageHiddenForModeration, + } = getEventDisplayInfo(mxEvent, false /* Replies are never hidden, so this should be fine */); // This shouldn't happen: the caller should check we support this type // before trying to instantiate us if (!hasRenderer) { @@ -177,7 +179,7 @@ export default class ReplyTile extends React.PureComponent { highlightLink: this.props.highlightLink, onHeightChanged: this.props.onHeightChanged, permalinkCreator: this.props.permalinkCreator, - }) } + }, false /* showHiddenEvents shouldn't be relevant */) } ); diff --git a/src/components/views/rooms/SearchResultTile.tsx b/src/components/views/rooms/SearchResultTile.tsx index a3d646aed3e..d4bb791aa86 100644 --- a/src/components/views/rooms/SearchResultTile.tsx +++ b/src/components/views/rooms/SearchResultTile.tsx @@ -78,7 +78,7 @@ export default class SearchResultTile extends React.Component { highlights = this.props.searchHighlights; } - if (haveRendererForEvent(mxEv, this.context?.showHiddenEventsInTimeline)) { + if (haveRendererForEvent(mxEv, this.context?.showHiddenEvents)) { // do we need a date separator since the last event? const prevEv = timeline[j - 1]; // is this a continuation of the previous message? @@ -87,7 +87,7 @@ export default class SearchResultTile extends React.Component { shouldFormContinuation( prevEv, mxEv, - this.context?.showHiddenEventsInTimeline, + this.context?.showHiddenEvents, threadsEnabled, TimelineRenderingType.Search, ); @@ -102,7 +102,7 @@ export default class SearchResultTile extends React.Component { !shouldFormContinuation( mxEv, nextEv, - this.context?.showHiddenEventsInTimeline, + this.context?.showHiddenEvents, threadsEnabled, TimelineRenderingType.Search, ) diff --git a/src/contexts/RoomContext.ts b/src/contexts/RoomContext.ts index 631b849e013..9c44553a983 100644 --- a/src/contexts/RoomContext.ts +++ b/src/contexts/RoomContext.ts @@ -54,7 +54,7 @@ const RoomContext = createContext({ showTwelveHourTimestamps: false, readMarkerInViewThresholdMs: 3000, readMarkerOutOfViewThresholdMs: 30000, - showHiddenEventsInTimeline: false, + showHiddenEvents: false, showReadReceipts: true, showRedactions: true, showJoinLeaves: true, diff --git a/src/events/EventTileFactory.tsx b/src/events/EventTileFactory.tsx index 3f470a19b61..7beda8cc305 100644 --- a/src/events/EventTileFactory.tsx +++ b/src/events/EventTileFactory.tsx @@ -141,19 +141,25 @@ const SINGULAR_STATE_EVENTS = new Set([ * Find an event tile factory for the given conditions. * @param mxEvent The event. * @param cli The matrix client to reference when needed. + * @param showHiddenEvents Whether hidden events should be shown. * @param asHiddenEv When true, treat the event as always hidden. * @returns The factory, or falsy if not possible. */ -export function pickFactory(mxEvent: MatrixEvent, cli: MatrixClient, asHiddenEv?: boolean): Optional { +export function pickFactory( + mxEvent: MatrixEvent, + cli: MatrixClient, + showHiddenEvents: boolean, + asHiddenEv?: boolean, +): Optional { const evType = mxEvent.getType(); // cache this to reduce call stack execution hits // Note: we avoid calling SettingsStore unless absolutely necessary - this code is on the critical path. - if (asHiddenEv && SettingsStore.getValue("showHiddenEventsInTimeline")) { + if (asHiddenEv && showHiddenEvents) { return JSONEventFactory; } - const noEventFactoryFactory: (() => Optional) = () => SettingsStore.getValue("showHiddenEventsInTimeline") + const noEventFactoryFactory: (() => Optional) = () => showHiddenEvents ? JSONEventFactory : undefined; // just don't render things that we shouldn't render @@ -242,17 +248,19 @@ export function pickFactory(mxEvent: MatrixEvent, cli: MatrixClient, asHiddenEv? * Render an event as a tile * @param renderType The render type. Used to inform properties given to the eventual component. * @param props The properties to provide to the eventual component. + * @param showHiddenEvents Whether hidden events should be shown. * @param cli Optional client instance to use, otherwise the default MatrixClientPeg will be used. * @returns The tile as JSX, or falsy if unable to render. */ export function renderTile( renderType: TimelineRenderingType, props: EventTileTypeProps, + showHiddenEvents: boolean, cli?: MatrixClient, ): Optional { cli = cli ?? MatrixClientPeg.get(); // because param defaults don't do the correct thing - const factory = pickFactory(props.mxEvent, cli); + const factory = pickFactory(props.mxEvent, cli, showHiddenEvents); if (!factory) return undefined; // Note that we split off the ones we actually care about here just to be sure that we're @@ -316,16 +324,18 @@ export function renderTile( /** * A version of renderTile() specifically for replies. * @param props The properties to specify on the eventual object. + * @param showHiddenEvents Whether hidden events should be shown. * @param cli Optional client instance to use, otherwise the default MatrixClientPeg will be used. * @returns The tile as JSX, or falsy if unable to render. */ export function renderReplyTile( props: EventTileTypeProps, + showHiddenEvents: boolean, cli?: MatrixClient, ): Optional { cli = cli ?? MatrixClientPeg.get(); // because param defaults don't do the correct thing - const factory = pickFactory(props.mxEvent, cli); + const factory = pickFactory(props.mxEvent, cli, showHiddenEvents); if (!factory) return undefined; // See renderTile() for why we split off so much @@ -367,7 +377,7 @@ export function isMessageEvent(ev: MatrixEvent): boolean { return (messageTypes.includes(ev.getType() as EventType)) || M_POLL_START.matches(ev.getType()); } -export function haveRendererForEvent(mxEvent: MatrixEvent, showHiddenEvents?: boolean): boolean { +export function haveRendererForEvent(mxEvent: MatrixEvent, showHiddenEvents: boolean): boolean { // Only show "Message deleted" tile for plain message events, encrypted events, // and state events as they'll likely still contain enough keys to be relevant. if (mxEvent.isRedacted() && !mxEvent.isEncrypted() && !isMessageEvent(mxEvent) && !mxEvent.isState()) { @@ -377,7 +387,7 @@ export function haveRendererForEvent(mxEvent: MatrixEvent, showHiddenEvents?: bo // No tile for replacement events since they update the original tile if (mxEvent.isRelation(RelationType.Replace)) return false; - const handler = pickFactory(mxEvent, MatrixClientPeg.get()); + const handler = pickFactory(mxEvent, MatrixClientPeg.get(), showHiddenEvents); if (!handler) return false; if (handler === TextualEventFactory) { return hasText(mxEvent, showHiddenEvents); diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index bd34297161b..2de76013636 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -187,6 +187,8 @@ export const SETTINGS: {[setting: string]: ISetting} = { "feature_msc3531_hide_messages_pending_moderation": { isFeature: true, labsGroup: LabGroup.Moderation, + // Requires a reload since this setting is cached in EventUtils + controller: new ReloadOnChangeController(), displayName: _td("Let moderators hide messages pending moderation."), supportedLevels: LEVELS_FEATURE, default: false, diff --git a/src/utils/EventRenderingUtils.ts b/src/utils/EventRenderingUtils.ts index fd6e518c4f2..a44db854fe2 100644 --- a/src/utils/EventRenderingUtils.ts +++ b/src/utils/EventRenderingUtils.ts @@ -25,7 +25,7 @@ import { haveRendererForEvent, JitsiEventFactory, JSONEventFactory, pickFactory import { MatrixClientPeg } from "../MatrixClientPeg"; import { getMessageModerationState, MessageModerationState } from "./EventUtils"; -export function getEventDisplayInfo(mxEvent: MatrixEvent, hideEvent?: boolean): { +export function getEventDisplayInfo(mxEvent: MatrixEvent, showHiddenEvents: boolean, hideEvent?: boolean): { isInfoMessage: boolean; hasRenderer: boolean; isBubbleMessage: boolean; @@ -52,7 +52,7 @@ export function getEventDisplayInfo(mxEvent: MatrixEvent, hideEvent?: boolean): } // TODO: Thread a MatrixClient through to here - let factory = pickFactory(mxEvent, MatrixClientPeg.get()); + let factory = pickFactory(mxEvent, MatrixClientPeg.get(), showHiddenEvents); // Info messages are basically information about commands processed on a room let isBubbleMessage = ( @@ -92,11 +92,11 @@ export function getEventDisplayInfo(mxEvent: MatrixEvent, hideEvent?: boolean): // source tile when there's no regular tile for an event and also for // replace relations (which otherwise would display as a confusing // duplicate of the thing they are replacing). - if (hideEvent || !haveRendererForEvent(mxEvent)) { + if (hideEvent || !haveRendererForEvent(mxEvent, showHiddenEvents)) { // forcefully ask for a factory for a hidden event (hidden event // setting is checked internally) // TODO: Thread a MatrixClient through to here - factory = pickFactory(mxEvent, MatrixClientPeg.get(), true); + factory = pickFactory(mxEvent, MatrixClientPeg.get(), showHiddenEvents, true); if (factory === JSONEventFactory) { isBubbleMessage = false; // Reuse info message avatar and sender profile styling diff --git a/src/utils/EventUtils.ts b/src/utils/EventUtils.ts index afcfc411398..4b626397f93 100644 --- a/src/utils/EventUtils.ts +++ b/src/utils/EventUtils.ts @@ -151,6 +151,16 @@ export enum MessageModerationState { SEE_THROUGH_FOR_CURRENT_USER = "SEE_THROUGH_FOR_CURRENT_USER", } +// This is lazily initialized and cached since getMessageModerationState needs it, +// and is called on timeline rendering hot-paths +let msc3531Enabled: boolean | null = null; +const getMsc3531Enabled = (): boolean => { + if (msc3531Enabled === null) { + msc3531Enabled = SettingsStore.getValue("feature_msc3531_hide_messages_pending_moderation"); + } + return msc3531Enabled; +}; + /** * Determine whether a message should be displayed as hidden pending moderation. * @@ -160,7 +170,7 @@ export enum MessageModerationState { export function getMessageModerationState(mxEvent: MatrixEvent, client?: MatrixClient): MessageModerationState { client = client ?? MatrixClientPeg.get(); // because param defaults don't do the correct thing - if (!SettingsStore.getValue("feature_msc3531_hide_messages_pending_moderation")) { + if (!getMsc3531Enabled()) { return MessageModerationState.VISIBLE_FOR_ALL; } const visibility = mxEvent.messageVisibility(); diff --git a/src/utils/exportUtils/HtmlExport.tsx b/src/utils/exportUtils/HtmlExport.tsx index 89ac6146ca3..59d864b6c72 100644 --- a/src/utils/exportUtils/HtmlExport.tsx +++ b/src/utils/exportUtils/HtmlExport.tsx @@ -407,7 +407,7 @@ export default class HTMLExporter extends Exporter { total: events.length, }), false, true); if (this.cancelled) return this.cleanUp(); - if (!haveRendererForEvent(event)) continue; + if (!haveRendererForEvent(event, false)) continue; content += this.needsDateSeparator(event, prevEvent) ? this.getDateSeparator(event) : ""; const shouldBeJoined = !this.needsDateSeparator(event, prevEvent) && diff --git a/src/utils/exportUtils/JSONExport.ts b/src/utils/exportUtils/JSONExport.ts index 673420327e7..b0b4b330b06 100644 --- a/src/utils/exportUtils/JSONExport.ts +++ b/src/utils/exportUtils/JSONExport.ts @@ -85,7 +85,7 @@ export default class JSONExporter extends Exporter { total: events.length, }), false, true); if (this.cancelled) return this.cleanUp(); - if (!haveRendererForEvent(event)) continue; + if (!haveRendererForEvent(event, false)) continue; this.messages.push(await this.getJSONString(event)); } return this.createJSONString(); diff --git a/src/utils/exportUtils/PlainTextExport.ts b/src/utils/exportUtils/PlainTextExport.ts index d41d06d35c5..e86c56e9f4c 100644 --- a/src/utils/exportUtils/PlainTextExport.ts +++ b/src/utils/exportUtils/PlainTextExport.ts @@ -112,7 +112,7 @@ export default class PlainTextExporter extends Exporter { total: events.length, }), false, true); if (this.cancelled) return this.cleanUp(); - if (!haveRendererForEvent(event)) continue; + if (!haveRendererForEvent(event, false)) continue; const textForEvent = await this.plainTextForEvent(event); content += textForEvent && `${new Date(event.getTs()).toLocaleString()} - ${textForEvent}\n`; } diff --git a/test/components/structures/MessagePanel-test.js b/test/components/structures/MessagePanel-test.js index 1471c52eb87..3c774290005 100644 --- a/test/components/structures/MessagePanel-test.js +++ b/test/components/structures/MessagePanel-test.js @@ -276,6 +276,30 @@ describe('MessagePanel', function() { }), ]; } + + function mkMixedHiddenAndShownEvents() { + const roomId = "!room:id"; + const userId = "@alice:example.org"; + const ts0 = Date.now(); + + return [ + TestUtilsMatrix.mkMessage({ + event: true, + room: roomId, + user: userId, + ts: ts0, + }), + TestUtilsMatrix.mkEvent({ + event: true, + type: "org.example.a_hidden_event", + room: roomId, + user: userId, + content: {}, + ts: ts0 + 1, + }), + ]; + } + function isReadMarkerVisible(rmContainer) { return rmContainer && rmContainer.children.length > 0; } @@ -594,6 +618,21 @@ describe('MessagePanel', function() { expect(els.first().prop("events").length).toEqual(5); expect(els.last().prop("events").length).toEqual(5); }); + + // We test this because setting lookups can be *slow*, and we don't want + // them to happen in this code path + it("doesn't lookup showHiddenEventsInTimeline while rendering", () => { + // We're only interested in the setting lookups that happen on every render, + // rather than those happening on first mount, so let's get those out of the way + const res = mount(); + + // Set up our spy and re-render with new events + const settingsSpy = jest.spyOn(SettingsStore, "getValue").mockClear(); + res.setProps({ events: mkMixedHiddenAndShownEvents() }); + + expect(settingsSpy).not.toHaveBeenCalledWith("showHiddenEventsInTimeline"); + settingsSpy.mockRestore(); + }); }); describe("shouldFormContinuation", () => { diff --git a/test/components/views/rooms/MessageComposerButtons-test.tsx b/test/components/views/rooms/MessageComposerButtons-test.tsx index c94323e1c33..4abe3e36373 100644 --- a/test/components/views/rooms/MessageComposerButtons-test.tsx +++ b/test/components/views/rooms/MessageComposerButtons-test.tsx @@ -227,7 +227,7 @@ function createRoomState(room: Room, narrow: boolean): IRoomState { showTwelveHourTimestamps: false, readMarkerInViewThresholdMs: 3000, readMarkerOutOfViewThresholdMs: 30000, - showHiddenEventsInTimeline: false, + showHiddenEvents: false, showReadReceipts: true, showRedactions: true, showJoinLeaves: true, diff --git a/test/components/views/rooms/SendMessageComposer-test.tsx b/test/components/views/rooms/SendMessageComposer-test.tsx index 30ecd31586b..5ab7a1705e5 100644 --- a/test/components/views/rooms/SendMessageComposer-test.tsx +++ b/test/components/views/rooms/SendMessageComposer-test.tsx @@ -73,7 +73,7 @@ describe('', () => { showTwelveHourTimestamps: false, readMarkerInViewThresholdMs: 3000, readMarkerOutOfViewThresholdMs: 30000, - showHiddenEventsInTimeline: false, + showHiddenEvents: false, showReadReceipts: true, showRedactions: true, showJoinLeaves: true,