From 59ccc1111a359a745847c9e0add54e057dc30c94 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Wed, 25 Feb 2026 15:53:30 +0000 Subject: [PATCH] Revert 'Hide the names of banned users behind a spoiler tag#32424' (#32635) * Revert 'Hide the names of banned users behind a spoiler tag#32424' * Empty commit to trigger test re-run --- apps/web/src/TextForEvent.tsx | 27 ++------ .../views/elements/EventListSummary.tsx | 29 ++------ apps/web/src/i18n/strings/en_EN.json | 6 +- .../web/test/unit-tests/TextForEvent-test.tsx | 69 ------------------- .../__snapshots__/MessagePanel-test.tsx.snap | 4 +- .../views/elements/EventListSummary-test.tsx | 22 +----- 6 files changed, 13 insertions(+), 144 deletions(-) diff --git a/apps/web/src/TextForEvent.tsx b/apps/web/src/TextForEvent.tsx index b9ea96dcd8..1f326663b7 100644 --- a/apps/web/src/TextForEvent.tsx +++ b/apps/web/src/TextForEvent.tsx @@ -39,7 +39,6 @@ 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(); @@ -108,7 +107,7 @@ function textForMemberEvent( client: MatrixClient, allowJSX: boolean, showHiddenEvents?: boolean, -): (() => Renderable) | null { +): (() => string) | 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()); @@ -134,26 +133,10 @@ function textForMemberEvent( } } case KnownMembership.Ban: - if (allowJSX) { - return reason - ? () => - _t( - "timeline|m.room.member|ban_reason_spoiler", - { senderName, reason }, - { user: () => {targetName} }, - ) - : () => - _t( - "timeline|m.room.member|ban_spoiler", - { senderName }, - { user: () => {targetName} }, - ); - } - - return reason - ? () => _t("timeline|m.room.member|ban_reason", { senderName, reason }) - : () => _t("timeline|m.room.member|ban", { senderName }); - + return () => + reason + ? _t("timeline|m.room.member|ban_reason", { senderName, targetName, reason }) + : _t("timeline|m.room.member|ban", { senderName, targetName }); case KnownMembership.Join: if (prevContent && prevContent.membership === KnownMembership.Join) { const modDisplayname = getModification(prevContent.displayname, content.displayname); diff --git a/apps/web/src/components/views/elements/EventListSummary.tsx b/apps/web/src/components/views/elements/EventListSummary.tsx index 9af2231436..86a5ce5776 100644 --- a/apps/web/src/components/views/elements/EventListSummary.tsx +++ b/apps/web/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 ReactElement, type ComponentProps, type ReactNode } from "react"; +import React, { 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,7 +25,6 @@ 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); @@ -223,15 +222,7 @@ export default class EventListSummary extends React.Component { ): ReactNode { const summaries = orderedTransitionSequences.map((transitions) => { const userNames = eventAggregates[transitions]; - let spoileredUserNames: ReactElement[]; - - if (containsBanned(transitions)) { - spoileredUserNames = userNames.map((u) => {u}); - } else { - spoileredUserNames = userNames.map((u) => <>{u}); - } - - const nameList = this.renderNameList(spoileredUserNames); + const nameList = this.renderNameList(userNames); const splitTransitions = transitions.split(SEP) as TransitionType[]; @@ -243,11 +234,7 @@ export default class EventListSummary extends React.Component { const coalescedTransitions = EventListSummary.coalesceRepeatedTransitions(canonicalTransitions); const descs = coalescedTransitions.map((t) => { - return EventListSummary.getDescriptionForTransition( - t.transitionType, - spoileredUserNames.length, - t.repeats, - ); + return EventListSummary.getDescriptionForTransition(t.transitionType, userNames.length, t.repeats); }); const desc = formatList(descs); @@ -268,7 +255,7 @@ export default class EventListSummary extends React.Component { * more items in `users` than `this.props.summaryLength`, which is the number of names * included before "and [n] others". */ - private renderNameList(users: ReactElement[]): ReactElement { + private renderNameList(users: string[]): string { return formatList(users, this.props.summaryLength); } @@ -631,11 +618,3 @@ export default class EventListSummary extends React.Component { ); } } - -/** - * 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}`); -} diff --git a/apps/web/src/i18n/strings/en_EN.json b/apps/web/src/i18n/strings/en_EN.json index 907b928d1c..f31931b7b0 100644 --- a/apps/web/src/i18n/strings/en_EN.json +++ b/apps/web/src/i18n/strings/en_EN.json @@ -3483,10 +3483,8 @@ "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 a user", - "ban_reason": "%(senderName)s banned a user: %(reason)s", - "ban_reason_spoiler": "%(senderName)s banned : %(reason)s", - "ban_spoiler": "%(senderName)s banned ", + "ban": "%(senderName)s banned %(targetName)s", + "ban_reason": "%(senderName)s banned %(targetName)s: %(reason)s", "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", diff --git a/apps/web/test/unit-tests/TextForEvent-test.tsx b/apps/web/test/unit-tests/TextForEvent-test.tsx index c6c23bffa2..076972e646 100644 --- a/apps/web/test/unit-tests/TextForEvent-test.tsx +++ b/apps/web/test/unit-tests/TextForEvent-test.tsx @@ -20,7 +20,6 @@ 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"; @@ -29,7 +28,6 @@ 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", () => ({ @@ -564,50 +562,6 @@ 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); - - expect(textForEvent(banEventWithReason(), mockClient, true)).toEqual( - - Admin banned Bad User: bad behaviour - , - ); - }); - - 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); - - 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); - - expect(textForEvent(banEvent(), mockClient, true)).toEqual( - - @admin:example.com banned @bad_name:bad_server.co - , - ); - }); - - 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); - - expect(textForEvent(banEvent(), mockClient)).toEqual("@admin:example.com banned a user"); - }); }); describe("textForJoinRulesEvent()", () => { @@ -763,26 +717,3 @@ 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", - }); -} diff --git a/apps/web/test/unit-tests/components/structures/__snapshots__/MessagePanel-test.tsx.snap b/apps/web/test/unit-tests/components/structures/__snapshots__/MessagePanel-test.tsx.snap index 5bb266d60d..f31612ae13 100644 --- a/apps/web/test/unit-tests/components/structures/__snapshots__/MessagePanel-test.tsx.snap +++ b/apps/web/test/unit-tests/components/structures/__snapshots__/MessagePanel-test.tsx.snap @@ -120,9 +120,7 @@ exports[`MessagePanel should handle lots of membership events quickly 1`] = ` - - @user:id made no changes 100 times - + @user:id made no changes 100 times diff --git a/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx b/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx index 921407781a..2f2c409db6 100644 --- a/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx +++ b/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx @@ -265,12 +265,7 @@ 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 () { @@ -314,14 +309,9 @@ 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('user_1'); }); it("handles multiple users following the same sequence of memberships", function () { @@ -371,14 +361,9 @@ 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('user_1'); }); it("handles many users following the same sequence of memberships", function () { @@ -408,14 +393,9 @@ 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('user_0'); }); it("correctly orders sequences of transitions by the order of their first event", function () {