Do not hide media from your own user by default (#29797)

* Always show media from your own user

* Update usages of useMediaVisible

* lint

* Add a test for HideActionButton

* Improve docs

* Document the event

* fixup test

* Allow users to hide their own media if they wish.

* Update tests

* remove a check\

* tweak

* tweak

playwright/e2e/timeline/timeline.spec.ts

@@ -908,23 +908,37 @@ test.describe("Timeline", () => {
             });
         });
 
-        test("should be able to hide an image", { tag: "@screenshot" }, async ({ page, app, room, context }) => {
+        test(
-            await app.viewRoomById(room.roomId);
+            "should be able to hide an image",
-            await sendImage(app.client, room.roomId, NEW_AVATAR);
+            { tag: "@screenshot" },
-            await app.timeline.scrollToBottom();
+            async ({ page, app, homeserver, room, context }) => {
-            const imgTile = page.locator(".mx_MImageBody").first();
+                await app.viewRoomById(room.roomId);
-            await expect(imgTile).toBeVisible();
-            await imgTile.hover();
-            await page.getByRole("button", { name: "Hide" }).click();
 
-            // Check that the image is now hidden.
+                const bot = new Bot(page, homeserver, {});
-            await expect(page.getByRole("button", { name: "Show image" })).toBeVisible();
+                await bot.prepareClient();
-        });
+                await app.client.inviteUser(room.roomId, bot.credentials.userId);
 
-        test("should be able to hide a video", async ({ page, app, room, context }) => {
+                await sendImage(bot, room.roomId, NEW_AVATAR);
+                await app.timeline.scrollToBottom();
+                const imgTile = page.locator(".mx_MImageBody").first();
+                await expect(imgTile).toBeVisible();
+                await imgTile.hover();
+                await page.getByRole("button", { name: "Hide" }).click();
+
+                // Check that the image is now hidden.
+                await expect(page.getByRole("button", { name: "Show image" })).toBeVisible();
+            },
+        );
+
+        test("should be able to hide a video", async ({ page, app, homeserver, room, context }) => {
             await app.viewRoomById(room.roomId);
-            const upload = await app.client.uploadContent(VIDEO_FILE, { name: "bbb.webm", type: "video/webm" });
+
-            await app.client.sendEvent(room.roomId, null, "m.room.message" as EventType, {
+            const bot = new Bot(page, homeserver, {});
+            await bot.prepareClient();
+            await app.client.inviteUser(room.roomId, bot.credentials.userId);
+
+            const upload = await bot.uploadContent(VIDEO_FILE, { name: "bbb.webm", type: "video/webm" });
+            await bot.sendEvent(room.roomId, null, "m.room.message" as EventType, {
                 msgtype: "m.video" as MsgType,
                 body: "bbb.webm",
                 url: upload.content_uri,

src/components/views/messages/EventContentBody.tsx

@@ -151,7 +151,7 @@ interface Props extends ReplacerOptions {
 const EventContentBody = memo(
     ({ as, mxEvent, stripReply, content, linkify, highlights, includeDir = true, ref, ...options }: Props) => {
         const enableBigEmoji = useSettingValue("TextualBody.enableBigEmoji");
-        const [mediaIsVisible] = useMediaVisible(mxEvent?.getId(), mxEvent?.getRoomId());
+        const [mediaIsVisible] = useMediaVisible(mxEvent);
 
         const replacer = useReplacer(content, mxEvent, options);
         const linkifyOptions = useMemo(

src/components/views/messages/HideActionButton.tsx

@@ -25,7 +25,7 @@ interface IProps {
  * Quick action button for marking a media event as hidden.
  */
 export const HideActionButton: React.FC<IProps> = ({ mxEvent }) => {
-    const [mediaIsVisible, setVisible] = useMediaVisible(mxEvent.getId(), mxEvent.getRoomId());
+    const [mediaIsVisible, setVisible] = useMediaVisible(mxEvent);
 
     if (!mediaIsVisible) {
         return;

src/components/views/messages/MImageBody.tsx

@@ -686,7 +686,7 @@ export class MImageBodyInner extends React.Component<IProps, IState> {
 
 // Wrap MImageBody component so we can use a hook here.
 const MImageBody: React.FC<IBodyProps> = (props) => {
-    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent.getId(), props.mxEvent.getRoomId());
+    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent);
     return <MImageBodyInner mediaVisible={mediaVisible} setMediaVisible={setVisible} {...props} />;
 };
 

src/components/views/messages/MImageReplyBody.tsx

@@ -38,7 +38,7 @@ class MImageReplyBodyInner extends MImageBodyInner {
     }
 }
 const MImageReplyBody: React.FC<IBodyProps> = (props) => {
-    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent.getId(), props.mxEvent.getRoomId());
+    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent);
     return <MImageReplyBodyInner mediaVisible={mediaVisible} setMediaVisible={setVisible} {...props} />;
 };
 

src/components/views/messages/MStickerBody.tsx

@@ -20,7 +20,7 @@ class MStickerBodyInner extends MImageBodyInner {
     protected onClick = (ev: React.MouseEvent): void => {
         ev.preventDefault();
         if (!this.props.mediaVisible) {
-            this.props.setMediaVisible?.(true);
+            this.props.setMediaVisible(true);
         }
     };
 
@@ -79,7 +79,7 @@ class MStickerBodyInner extends MImageBodyInner {
 }
 
 const MStickerBody: React.FC<IBodyProps> = (props) => {
-    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent.getId(), props.mxEvent.getRoomId());
+    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent);
     return <MStickerBodyInner mediaVisible={mediaVisible} setMediaVisible={setVisible} {...props} />;
 };
 

src/components/views/messages/MVideoBody.tsx

@@ -342,7 +342,7 @@ class MVideoBodyInner extends React.PureComponent<IProps, IState> {
 
 // Wrap MVideoBody component so we can use a hook here.
 const MVideoBody: React.FC<IBodyProps> = (props) => {
-    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent.getId(), props.mxEvent.getRoomId());
+    const [mediaVisible, setVisible] = useMediaVisible(props.mxEvent);
     return <MVideoBodyInner mediaVisible={mediaVisible} setMediaVisible={setVisible} {...props} />;
 };
 

src/components/views/rooms/LinkPreviewGroup.tsx

@@ -30,7 +30,7 @@ interface IProps {
 const LinkPreviewGroup: React.FC<IProps> = ({ links, mxEvent, onCancelClick }) => {
     const cli = useContext(MatrixClientContext);
     const [expanded, toggleExpanded] = useStateToggle();
-    const [mediaVisible] = useMediaVisible(mxEvent.getId(), mxEvent.getRoomId());
+    const [mediaVisible] = useMediaVisible(mxEvent);
 
     const ts = mxEvent.getTs();
     const previews = useAsyncMemo<[string, IPreviewUrlResponse][]>(

src/hooks/useMediaVisible.ts

@@ -6,7 +6,7 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { useCallback } from "react";
-import { JoinRule } from "matrix-js-sdk/src/matrix";
+import { JoinRule, type MatrixEvent } from "matrix-js-sdk/src/matrix";
 
 import { SettingLevel } from "../settings/SettingLevel";
 import { useSettingValue } from "./useSettings";
@@ -19,14 +19,25 @@ const PRIVATE_JOIN_RULES: JoinRule[] = [JoinRule.Invite, JoinRule.Knock, JoinRul
 
 /**
  * Should the media event be visible in the client, or hidden.
- * @param eventId The eventId of the media event.
+ *
- * @returns A boolean describing the hidden status, and a function to set the visiblity.
+ * This function uses the `mediaPreviewConfig` setting to determine the rules for the room
+ * along with the `showMediaEventIds` setting for specific events.
+ *
+ * A function may be provided to alter the visible state.
+ *
+ * @param The event that contains the media. If not provided, the global rule is used.
+ *
+ * @returns Returns a tuple of:
+ *          A boolean describing the hidden status.
+ *          A function to show or hide the event.
  */
-export function useMediaVisible(eventId?: string, roomId?: string): [boolean, (visible: boolean) => void] {
+export function useMediaVisible(mxEvent?: MatrixEvent): [boolean, (visible: boolean) => void] {
-    const mediaPreviewSetting = useSettingValue("mediaPreviewConfig", roomId);
+    const eventId = mxEvent?.getId();
+    const mediaPreviewSetting = useSettingValue("mediaPreviewConfig", mxEvent?.getRoomId());
     const client = useMatrixClientContext();
     const eventVisibility = useSettingValue("showMediaEventIds");
-    const joinRule = useRoomState(client.getRoom(roomId) ?? undefined, (state) => state.getJoinRule());
+    const room = client.getRoom(mxEvent?.getRoomId()) ?? undefined;
+    const joinRule = useRoomState(room, (state) => state.getJoinRule());
     const setMediaVisible = useCallback(
         (visible: boolean) => {
             SettingsStore.setValue("showMediaEventIds", null, SettingLevel.DEVICE, {
@@ -43,6 +54,9 @@ export function useMediaVisible(eventId?: string, roomId?: string): [boolean, (v
     // Always prefer the explicit per-event user preference here.
     if (explicitEventVisiblity !== undefined) {
         return [explicitEventVisiblity, setMediaVisible];
+    } else if (mxEvent?.getSender() === client.getUserId()) {
+        // If this event is ours and we've not set an explicit visibility, default to on.
+        return [true, setMediaVisible];
     } else if (mediaPreviewSetting.media_previews === MediaPreviewValue.Off) {
         return [false, setMediaVisible];
     } else if (mediaPreviewSetting.media_previews === MediaPreviewValue.On) {

src/utils/MediaEventHelper.ts

@@ -117,7 +117,7 @@ export class MediaEventHelper implements IDestroyable {
     /**
      * Determine if the media event in question supports being hidden in the timeline.
      * @param event Any matrix event.
-     * @returns `true` if the media can be hidden, otherwise false.
+     * @returns `true` if the media can be hidden, otherwise `false`.
      */
     public static canHide(event: MatrixEvent): boolean {
         if (!event) return false;

test/unit-tests/components/views/messages/HideActionButton-test.tsx

@@ -48,6 +48,7 @@ describe("HideActionButton", () => {
     beforeEach(() => {
         cli = getMockClientWithEventEmitter({
             getRoom: jest.fn(),
+            getUserId: jest.fn(),
         });
     });
     afterEach(() => {

test/unit-tests/components/views/messages/MImageBody-test.tsx

@@ -35,10 +35,11 @@ jest.mock("matrix-encrypt-attachment", () => ({
 }));
 
 describe("<MImageBody/>", () => {
-    const userId = "@user:server";
+    const ourUserId = "@user:server";
+    const senderUserId = "@other_use:server";
     const deviceId = "DEADB33F";
     const cli = getMockClientWithEventEmitter({
-        ...mockClientMethodsUser(userId),
+        ...mockClientMethodsUser(ourUserId),
         ...mockClientMethodsServer(),
         ...mockClientMethodsDevice(deviceId),
         ...mockClientMethodsCrypto(),
@@ -62,7 +63,7 @@ describe("<MImageBody/>", () => {
     const encryptedMediaEvent = new MatrixEvent({
         event_id: "$foo:bar",
         room_id: "!room:server",
-        sender: userId,
+        sender: senderUserId,
         type: EventType.RoomMessage,
         content: {
             body: "alt for a test image",
@@ -201,7 +202,7 @@ describe("<MImageBody/>", () => {
 
         const event = new MatrixEvent({
             room_id: "!room:server",
-            sender: userId,
+            sender: senderUserId,
             type: EventType.RoomMessage,
             content: {
                 body: "alt for a test image",
@@ -254,7 +255,7 @@ describe("<MImageBody/>", () => {
 
         const event = new MatrixEvent({
             room_id: "!room:server",
-            sender: userId,
+            sender: senderUserId,
             type: EventType.RoomMessage,
             content: {
                 body: "alt for a test image",
@@ -281,7 +282,7 @@ describe("<MImageBody/>", () => {
     it("should show banner on hover", async () => {
         const event = new MatrixEvent({
             room_id: "!room:server",
-            sender: userId,
+            sender: senderUserId,
             type: EventType.RoomMessage,
             content: {
                 body: "alt for a test image",

test/unit-tests/components/views/messages/MVideoBody-test.tsx

@@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details.
 
 import React from "react";
 import { EventType, getHttpUriForMxc, type IContent, type MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix";
-import { fireEvent, render, screen, type RenderResult } from "jest-matrix-react";
+import { fireEvent, render, screen } from "jest-matrix-react";
 import fetchMock from "fetch-mock-jest";
 import { type MockedObject } from "jest-mock";
 
@@ -34,7 +34,8 @@ jest.mock("matrix-encrypt-attachment", () => ({
 }));
 
 describe("MVideoBody", () => {
-    const userId = "@user:server";
+    const ourUserId = "@user:server";
+    const senderUserId = "@other_use:server";
     const deviceId = "DEADB33F";
 
     const thumbUrl = "https://server/_matrix/media/v3/download/server/encrypted-poster";
@@ -42,7 +43,7 @@ describe("MVideoBody", () => {
 
     beforeEach(() => {
         cli = getMockClientWithEventEmitter({
-            ...mockClientMethodsUser(userId),
+            ...mockClientMethodsUser(ourUserId),
             ...mockClientMethodsServer(),
             ...mockClientMethodsDevice(deviceId),
             ...mockClientMethodsCrypto(),
@@ -67,7 +68,7 @@ describe("MVideoBody", () => {
 
     const encryptedMediaEvent = new MatrixEvent({
         room_id: "!room:server",
-        sender: userId,
+        sender: senderUserId,
         type: EventType.RoomMessage,
         event_id: "$foo:bar",
         content: {
@@ -86,10 +87,47 @@ describe("MVideoBody", () => {
         },
     });
 
-    it("does not crash when given a portrait image", () => {
+    it("does not crash when given portrait dimensions", () => {
         // Check for an unreliable crash caused by a fractional-sized
         // image dimension being used for a CanvasImageData.
-        const { asFragment } = makeMVideoBody(720, 1280);
+        const content: IContent = {
+            info: {
+                "w": 720,
+                "h": 1280,
+                "mimetype": "video/mp4",
+                "size": 2495675,
+                "thumbnail_file": {
+                    url: "",
+                    key: { alg: "", key_ops: [], kty: "", k: "", ext: true },
+                    iv: "",
+                    hashes: {},
+                    v: "",
+                },
+                "thumbnail_info": { mimetype: "" },
+                "xyz.amorgan.blurhash": "TrGl6bofof~paxWC?bj[oL%2fPj]",
+            },
+            url: "http://example.com",
+        };
+
+        const event = new MatrixEvent({
+            content,
+        });
+
+        const defaultProps: IBodyProps = {
+            mxEvent: event,
+            highlights: [],
+            highlightLink: "",
+            onMessageAllowed: jest.fn(),
+            permalinkCreator: {} as RoomPermalinkCreator,
+            mediaEventHelper: { media: { isEncrypted: false } } as MediaEventHelper,
+        };
+
+        const { asFragment } = render(
+            <MatrixClientContext.Provider value={cli}>
+                <MVideoBody {...defaultProps} />
+            </MatrixClientContext.Provider>,
+            withClientContextRenderOptions(cli),
+        );
         expect(asFragment()).toMatchSnapshot();
         // If we get here, we did not crash.
     });
@@ -153,50 +191,39 @@ describe("MVideoBody", () => {
 
             expect(fetchMock).toHaveFetched(thumbUrl);
         });
+
+        it("should download video if we were the sender", async () => {
+            fetchMock.getOnce(thumbUrl, { status: 200 });
+            const ourEncryptedMediaEvent = new MatrixEvent({
+                room_id: "!room:server",
+                sender: ourUserId,
+                type: EventType.RoomMessage,
+                event_id: "$foo:bar",
+                content: {
+                    body: "alt for a test video",
+                    info: {
+                        duration: 420,
+                        w: 40,
+                        h: 50,
+                        thumbnail_file: {
+                            url: "mxc://server/encrypted-poster",
+                        },
+                    },
+                    file: {
+                        url: "mxc://server/encrypted-image",
+                    },
+                },
+            });
+            const { asFragment } = render(
+                <MVideoBody
+                    mxEvent={ourEncryptedMediaEvent}
+                    mediaEventHelper={new MediaEventHelper(ourEncryptedMediaEvent)}
+                />,
+                withClientContextRenderOptions(cli),
+            );
+
+            expect(fetchMock).toHaveFetched(thumbUrl);
+            expect(asFragment()).toMatchSnapshot();
+        });
     });
 });
-
-function makeMVideoBody(w: number, h: number): RenderResult {
-    const content: IContent = {
-        info: {
-            "w": w,
-            "h": h,
-            "mimetype": "video/mp4",
-            "size": 2495675,
-            "thumbnail_file": {
-                url: "",
-                key: { alg: "", key_ops: [], kty: "", k: "", ext: true },
-                iv: "",
-                hashes: {},
-                v: "",
-            },
-            "thumbnail_info": { mimetype: "" },
-            "xyz.amorgan.blurhash": "TrGl6bofof~paxWC?bj[oL%2fPj]",
-        },
-        url: "http://example.com",
-    };
-
-    const event = new MatrixEvent({
-        content,
-    });
-
-    const defaultProps: IBodyProps = {
-        mxEvent: event,
-        highlights: [],
-        highlightLink: "",
-        onMessageAllowed: jest.fn(),
-        permalinkCreator: {} as RoomPermalinkCreator,
-        mediaEventHelper: { media: { isEncrypted: false } } as MediaEventHelper,
-    };
-
-    const mockClient = getMockClientWithEventEmitter({
-        mxcUrlToHttp: jest.fn(),
-        getRoom: jest.fn(),
-    });
-
-    return render(
-        <MatrixClientContext.Provider value={mockClient}>
-            <MVideoBody {...defaultProps} />
-        </MatrixClientContext.Provider>,
-    );
-}

test/unit-tests/components/views/messages/__snapshots__/MVideoBody-test.tsx.snap

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`MVideoBody does not crash when given a portrait image 1`] = `
+exports[`MVideoBody does not crash when given portrait dimensions 1`] = `
 <DocumentFragment>
   <span
     class="mx_MVideoBody"
@@ -48,3 +48,28 @@ exports[`MVideoBody should show poster for encrypted media before downloading it
   </span>
 </DocumentFragment>
 `;
+
+exports[`MVideoBody with video previews/thumbnails disabled should download video if we were the sender 1`] = `
+<DocumentFragment>
+  <span
+    class="mx_MVideoBody"
+  >
+    <div
+      class="mx_MVideoBody_container"
+      style="max-width: 40px; max-height: 50px;"
+    >
+      <video
+        class="mx_MVideoBody"
+        controls=""
+        controlslist="nodownload"
+        poster="https://server/_matrix/media/v3/download/server/encrypted-poster"
+        preload="none"
+        title="alt for a test video"
+      />
+      <div
+        style="width: 40px; height: 50px;"
+      />
+    </div>
+  </span>
+</DocumentFragment>
+`;

test/unit-tests/hooks/useMediaVisible-test.tsx

@@ -6,7 +6,7 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { act, renderHook, waitFor } from "jest-matrix-react";
-import { JoinRule, type MatrixClient, type Room } from "matrix-js-sdk/src/matrix";
+import { JoinRule, MatrixEvent, type MatrixClient, type Room } from "matrix-js-sdk/src/matrix";
 
 import { useMediaVisible } from "../../../src/hooks/useMediaVisible";
 import { createTestClient, mkStubRoom, withClientContextRenderOptions } from "../../test-utils";
@@ -22,8 +22,18 @@ describe("useMediaVisible", () => {
     let room: Room;
     const mediaPreviewConfig: MediaPreviewConfig = MediaPreviewConfigController.default;
 
-    function render() {
+    function render({ sender }: { sender?: string } = {}) {
-        return renderHook(() => useMediaVisible(EVENT_ID, ROOM_ID), withClientContextRenderOptions(matrixClient));
+        return renderHook(
+            () =>
+                useMediaVisible(
+                    new MatrixEvent({
+                        event_id: EVENT_ID,
+                        room_id: ROOM_ID,
+                        sender,
+                    }),
+                ),
+            withClientContextRenderOptions(matrixClient),
+        );
     }
     beforeEach(() => {
         matrixClient = createTestClient();
@@ -42,53 +52,62 @@ describe("useMediaVisible", () => {
         jest.restoreAllMocks();
     });
 
-    it("should display media by default", async () => {
+    it("should display media by default", () => {
-        const { result } = render();
+        const [visible] = render().result.current;
-        expect(result.current[0]).toEqual(true);
+        expect(visible).toEqual(true);
     });
 
-    it("should hide media when media previews are Off", async () => {
+    it("should hide media when media previews are Off", () => {
         mediaPreviewConfig.media_previews = MediaPreviewValue.Off;
-        const { result } = render();
+        const [visible] = render().result.current;
-        expect(result.current[0]).toEqual(false);
+        expect(visible).toEqual(false);
+    });
+
+    it("should always show media sent by us", () => {
+        mediaPreviewConfig.media_previews = MediaPreviewValue.Off;
+        const [visible] = render({ sender: matrixClient.getUserId()! }).result.current;
+        expect(visible).toEqual(true);
     });
 
     it.each([[JoinRule.Invite], [JoinRule.Knock], [JoinRule.Restricted]])(
         "should display media when media previews are Private and the join rule is %s",
-        async (rule) => {
+        (rule) => {
             mediaPreviewConfig.media_previews = MediaPreviewValue.Private;
             room.currentState.getJoinRule = jest.fn().mockReturnValue(rule);
-            const { result } = render();
+            const [visible] = render().result.current;
-            expect(result.current[0]).toEqual(true);
+            expect(visible).toEqual(true);
         },
     );
 
     it.each([[JoinRule.Public], ["anything_else"]])(
         "should hide media when media previews are Private and the join rule is %s",
-        async (rule) => {
+        (rule) => {
             mediaPreviewConfig.media_previews = MediaPreviewValue.Private;
             room.currentState.getJoinRule = jest.fn().mockReturnValue(rule);
-            const { result } = render();
+            const [visible] = render().result.current;
-            expect(result.current[0]).toEqual(false);
+            expect(visible).toEqual(false);
         },
     );
 
     it("should hide media after function is called", async () => {
         const { result } = render();
         expect(result.current[0]).toEqual(true);
+        expect(result.current[1]).toBeDefined();
         act(() => {
-            result.current[1](false);
+            result.current[1]!(false);
         });
         await waitFor(() => {
             expect(result.current[0]).toEqual(false);
         });
     });
+
     it("should show media after function is called", async () => {
         mediaPreviewConfig.media_previews = MediaPreviewValue.Off;
         const { result } = render();
         expect(result.current[0]).toEqual(false);
+        expect(result.current[1]).toBeDefined();
         act(() => {
-            result.current[1](true);
+            result.current[1]!(true);
         });
         await waitFor(() => {
             expect(result.current[0]).toEqual(true);