Hide the names of banned users behind a spoiler tag

Author: andybalaam

src/TextForEvent.tsx

@@ -39,6 +39,7 @@ import { highlightEvent, isLocationEvent } from "./utils/EventUtils";
 import { getSenderName } from "./utils/event/getSenderName";
 import PosthogTrackers from "./PosthogTrackers.ts";
 import { ElementCallEventType } from "./call-types.ts";
+import Spoiler from "./components/views/elements/Spoiler.tsx";
 
 function getRoomMemberDisplayname(client: MatrixClient, event: MatrixEvent, userId = event.getSender()): string {
     const roomId = event.getRoomId();
@@ -107,7 +108,7 @@ function textForMemberEvent(
     client: MatrixClient,
     allowJSX: boolean,
     showHiddenEvents?: boolean,
-): (() => string) | null {
+): (() => Renderable) | null {
     // XXX: SYJS-16 "sender is sometimes null for join messages"
     const senderName = ev.sender?.name || getRoomMemberDisplayname(client, ev);
     const targetName = ev.target?.name || getRoomMemberDisplayname(client, ev, ev.getStateKey());
@@ -133,10 +134,26 @@ function textForMemberEvent(
             }
         }
         case KnownMembership.Ban:
-            return () =>
-                reason
-                    ? _t("timeline|m.room.member|ban_reason", { senderName, targetName, reason })
-                    : _t("timeline|m.room.member|ban", { senderName, targetName });
+            if (allowJSX) {
+                return reason
+                    ? () =>
+                          _t(
+                              "timeline|m.room.member|ban_reason_spoiler",
+                              { senderName, reason },
+                              { user: () => <Spoiler>{targetName}</Spoiler> },
+                          )
+                    : () =>
+                          _t(
+                              "timeline|m.room.member|ban_spoiler",
+                              { senderName },
+                              { user: () => <Spoiler>{targetName}</Spoiler> },
+                          );
+            }
+
+            return reason
+                ? () => _t("timeline|m.room.member|ban_reason", { senderName, reason })
+                : () => _t("timeline|m.room.member|ban", { senderName });
+
         case KnownMembership.Join:
             if (prevContent && prevContent.membership === KnownMembership.Join) {
                 const modDisplayname = getModification(prevContent.displayname, content.displayname);

src/components/views/elements/EventListSummary.tsx

@@ -8,7 +8,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */
 
-import React, { type ComponentProps, type ReactNode } from "react";
+import React, { type ReactElement, type ComponentProps, type ReactNode } from "react";
 import { EventType, type MatrixEvent, MatrixEventEvent, type RoomMember } from "matrix-js-sdk/src/matrix";
 import { KnownMembership } from "matrix-js-sdk/src/types";
 import { throttle } from "lodash";
@@ -25,6 +25,7 @@ import AccessibleButton from "./AccessibleButton";
 import RoomContext from "../../../contexts/RoomContext";
 import { arrayHasDiff } from "../../../utils/arrays.ts";
 import { objectHasDiff } from "../../../utils/objects.ts";
+import Spoiler from "./Spoiler.tsx";
 
 const onPinnedMessagesClick = (): void => {
     RightPanelStore.instance.setCard({ phase: RightPanelPhases.PinnedMessages }, false);
@@ -222,7 +223,15 @@ export default class EventListSummary extends React.Component<Props, State> {
     ): ReactNode {
         const summaries = orderedTransitionSequences.map((transitions) => {
             const userNames = eventAggregates[transitions];
-            const nameList = this.renderNameList(userNames);
+            let spoileredUserNames: ReactElement[];
+
+            if (containsBanned(transitions)) {
+                spoileredUserNames = userNames.map((u) => <Spoiler key={u}>{u}</Spoiler>);
+            } else {
+                spoileredUserNames = userNames.map((u) => <>{u}</>);
+            }
+
+            const nameList = this.renderNameList(spoileredUserNames);
 
             const splitTransitions = transitions.split(SEP) as TransitionType[];
 
@@ -234,7 +243,11 @@ export default class EventListSummary extends React.Component<Props, State> {
             const coalescedTransitions = EventListSummary.coalesceRepeatedTransitions(canonicalTransitions);
 
             const descs = coalescedTransitions.map((t) => {
-                return EventListSummary.getDescriptionForTransition(t.transitionType, userNames.length, t.repeats);
+                return EventListSummary.getDescriptionForTransition(
+                    t.transitionType,
+                    spoileredUserNames.length,
+                    t.repeats,
+                );
             });
 
             const desc = formatList(descs);
@@ -255,7 +268,7 @@ export default class EventListSummary extends React.Component<Props, State> {
      * more items in `users` than `this.props.summaryLength`, which is the number of names
      * included before "and [n] others".
      */
-    private renderNameList(users: string[]): string {
+    private renderNameList(users: ReactElement[]): ReactElement {
         return formatList(users, this.props.summaryLength);
     }
 
@@ -618,3 +631,11 @@ export default class EventListSummary extends React.Component<Props, State> {
         );
     }
 }
+
+/**
+ * Returns true if the provided list comma-separated list of transitions
+ * contains an item "banned".
+ */
+function containsBanned(transitions: string): boolean {
+    return transitions.startsWith(TransitionType.Banned) || transitions.includes(`,${TransitionType.Banned}`);
+}

src/i18n/strings/en_EN.json

@@ -3483,8 +3483,10 @@
         "m.room.member": {
             "accepted_3pid_invite": "%(targetName)s accepted the invitation for %(displayName)s",
             "accepted_invite": "%(targetName)s accepted an invitation",
-            "ban": "%(senderName)s banned %(targetName)s",
-            "ban_reason": "%(senderName)s banned %(targetName)s: %(reason)s",
+            "ban": "%(senderName)s banned a user",
+            "ban_reason": "%(senderName)s banned a user: %(reason)s",
+            "ban_reason_spoiler": "%(senderName)s banned <user/>: %(reason)s",
+            "ban_spoiler": "%(senderName)s banned <user/>",
             "change_avatar": "%(senderName)s changed their profile picture",
             "change_name": "%(oldDisplayName)s changed their display name to %(displayName)s",
             "change_name_avatar": "%(oldDisplayName)s changed their display name and profile picture",

test/unit-tests/TextForEvent-test.ts test/unit-tests/TextForEvent-test.tsxtest/unit-tests/TextForEvent-test.ts renamed to test/unit-tests/TextForEvent-test.tsx

@@ -20,6 +20,7 @@ import { KnownMembership } from "matrix-js-sdk/src/types";
 import { render } from "jest-matrix-react";
 import { type ReactElement } from "react";
 import { type Mocked, mocked } from "jest-mock";
+import React from "react";
 
 import { hasText, textForEvent } from "../../src/TextForEvent";
 import SettingsStore from "../../src/settings/SettingsStore";
@@ -28,6 +29,7 @@ import { MatrixClientPeg } from "../../src/MatrixClientPeg";
 import UserIdentifierCustomisations from "../../src/customisations/UserIdentifier";
 import { getSenderName } from "../../src/utils/event/getSenderName";
 import { ElementCallEventType } from "../../src/call-types";
+import Spoiler from "../../src/components/views/elements/Spoiler";
 
 jest.mock("../../src/settings/SettingsStore");
 jest.mock("../../src/customisations/UserIdentifier", () => ({
@@ -562,6 +564,50 @@ describe("TextForEvent", () => {
                 ),
             ).toMatchInlineSnapshot(`"Member rejected the invitation: I don't want to be in this room."`);
         });
+
+        it("shows single-user bans with a spoiler on display name", () => {
+            mocked(mockClient.getRoom).mockReturnValue({
+                getMember: jest.fn().mockImplementation((userId) => {
+                    return { rawDisplayName: userId === "@admin:example.com" ? "Admin" : "Bad User" };
+                }),
+            } as unknown as Mocked<Room>);
+
+            expect(textForEvent(banEventWithReason(), mockClient, true)).toEqual(
+                <span>
+                    Admin banned <Spoiler>Bad User</Spoiler>: bad behaviour
+                </span>,
+            );
+        });
+
+        it("hides user name for single-user bans with reason when JSX is not allowed", () => {
+            mocked(mockClient.getRoom).mockReturnValue({
+                getMember: jest.fn().mockImplementation((userId) => {
+                    return { rawDisplayName: userId === "@admin:example.com" ? "Admin" : "Bad User" };
+                }),
+            } as unknown as Mocked<Room>);
+
+            expect(textForEvent(banEventWithReason(), mockClient)).toEqual("Admin banned a user: bad behaviour");
+        });
+
+        it("shows single-user bans with a spoiler on user ID", () => {
+            mocked(mockClient.getRoom).mockReturnValue({
+                getMember: jest.fn().mockReturnValue({ rawDisplayName: undefined }),
+            } as unknown as Mocked<Room>);
+
+            expect(textForEvent(banEvent(), mockClient, true)).toEqual(
+                <span>
+                    @admin:example.com banned <Spoiler>@bad_name:bad_server.co</Spoiler>
+                </span>,
+            );
+        });
+
+        it("hides user name for single-user bans when JSX is not allowed", () => {
+            mocked(mockClient.getRoom).mockReturnValue({
+                getMember: jest.fn().mockReturnValue({ rawDisplayName: undefined }),
+            } as unknown as Mocked<Room>);
+
+            expect(textForEvent(banEvent(), mockClient)).toEqual("@admin:example.com banned a user");
+        });
     });
 
     describe("textForJoinRulesEvent()", () => {
@@ -717,3 +763,26 @@ describe("TextForEvent", () => {
         });
     });
 });
+
+function banEvent(): MatrixEvent {
+    return new MatrixEvent({
+        type: "m.room.member",
+        sender: "@admin:example.com",
+        content: {
+            membership: KnownMembership.Ban,
+        },
+        state_key: "@bad_name:bad_server.co",
+    });
+}
+
+function banEventWithReason(): MatrixEvent {
+    return new MatrixEvent({
+        type: "m.room.member",
+        sender: "@admin:example.com",
+        content: {
+            membership: KnownMembership.Ban,
+            reason: "bad behaviour",
+        },
+        state_key: "@bad_name:bad_server.co",
+    });
+}

…/__snapshots__/TextForEvent-test.ts.snap …__snapshots__/TextForEvent-test.tsx.snaptest/unit-tests/__snapshots__/TextForEvent-test.ts.snap renamed to test/unit-tests/__snapshots__/TextForEvent-test.tsx.snap test/unit-tests/__snapshots__/TextForEvent-test.ts.snap renamed to test/unit-tests/__snapshots__/TextForEvent-test.tsx.snap

@@ -120,7 +120,9 @@ exports[`MessagePanel should handle lots of membership events quickly 1`] = `
               <span
                 class="mx_TextualEvent mx_GenericEventListSummary_summary"
               >
-                @user:id made no changes 100 times
+                <span>
+                  @user:id made no changes 100 times
+                </span>
               </span>
             </div>
           </div>

test/unit-tests/components/structures/__snapshots__/MessagePanel-test.tsx.snap

@@ -265,7 +265,12 @@ describe("EventListSummary", function () {
 
         const { container } = renderComponent(props);
         const summary = container.querySelector(".mx_GenericEventListSummary_summary");
+
+        // The sequence was summarised correctly
         expect(summary).toHaveTextContent("user_1 was unbanned, joined and left 7 times and was invited");
+
+        // And there is no spoiler on the user's name since they were not banned
+        expect(summary).not.toContainHTML("mx_EventTile_spoiler_content");
     });
 
     it("truncates multiple sequences of repetitions with other events between", function () {
@@ -309,9 +314,14 @@ describe("EventListSummary", function () {
 
         const { container } = renderComponent(props);
         const summary = container.querySelector(".mx_GenericEventListSummary_summary");
+
+        // The sequence was summarised correctly
         expect(summary).toHaveTextContent(
-            "user_1 was unbanned, joined and left 2 times, was banned, " + "joined and left 3 times and was invited",
+            "user_1 was unbanned, joined and left 2 times, was banned, joined and left 3 times and was invited",
         );
+
+        // And the banned user's name is hidden within a spoiler
+        expect(summary).toContainHTML('<span class="mx_EventTile_spoiler_content">user_1</span>');
     });
 
     it("handles multiple users following the same sequence of memberships", function () {
@@ -361,9 +371,14 @@ describe("EventListSummary", function () {
 
         const { container } = renderComponent(props);
         const summary = container.querySelector(".mx_GenericEventListSummary_summary");
+
+        // The sequence was summarised correctly
         expect(summary).toHaveTextContent(
             "user_1 and one other were unbanned, joined and left 2 times and were banned",
         );
+
+        // And the banned user's name is hidden within a spoiler
+        expect(summary).toContainHTML('<span class="mx_EventTile_spoiler_content">user_1</span>');
     });
 
     it("handles many users following the same sequence of memberships", function () {
@@ -393,9 +408,14 @@ describe("EventListSummary", function () {
 
         const { container } = renderComponent(props);
         const summary = container.querySelector(".mx_GenericEventListSummary_summary");
+
+        // The sequence was summarised correctly
         expect(summary).toHaveTextContent(
             "user_0 and 19 others were unbanned, joined and left 2 times and were banned",
         );
+
+        // And the banned user's name is hidden within a spoiler
+        expect(summary).toContainHTML('<span class="mx_EventTile_spoiler_content">user_0</span>');
     });
 
     it("correctly orders sequences of transitions by the order of their first event", function () {