Merge pull request #3780 from JephDiel/Download-Avatars-with-MC4039

Download avatars using the MC4039 Widget API

src/Avatar.test.tsx

@@ -9,14 +9,18 @@ import { afterEach, expect, test, vi } from "vitest";
 import { render, screen } from "@testing-library/react";
 import { type MatrixClient } from "matrix-js-sdk";
 import { type FC, type PropsWithChildren } from "react";
+import { type WidgetApi } from "matrix-widget-api";
 
 import { ClientContextProvider } from "./ClientContext";
 import { Avatar } from "./Avatar";
 import { mockMatrixRoomMember, mockRtcMembership } from "./utils/test";
+import { widget } from "./widget";
 
 const TestComponent: FC<
-  PropsWithChildren<{ client: MatrixClient; supportsThumbnails?: boolean }>
-> = ({ client, children, supportsThumbnails }) => {
+  PropsWithChildren<{
+    client: MatrixClient;
+  }>
+> = ({ client, children }) => {
   return (
     <ClientContextProvider
       value={{
@@ -24,7 +28,6 @@ const TestComponent: FC<
         disconnected: false,
         supportedFeatures: {
           reactions: true,
-          thumbnails: supportsThumbnails ?? true,
         },
         setClient: vi.fn(),
         authenticated: {
@@ -40,6 +43,12 @@ const TestComponent: FC<
   );
 };
 
+vi.mock("./widget", () => ({
+  widget: {
+    api: null, // Ideally we'd only mock this in the as a widget test so the whole module is otherwise null, but just nulling `api` by default works well enough
+  },
+}));
+
 afterEach(() => {
   vi.unstubAllGlobals();
 });
@@ -73,36 +82,7 @@ test("should just render a placeholder when the user has no avatar", () => {
   expect(client.mxcUrlToHttp).toBeCalledTimes(0);
 });
 
-test("should just render a placeholder when thumbnails are not supported", () => {
-  const client = vi.mocked<MatrixClient>({
-    getAccessToken: () => "my-access-token",
-    mxcUrlToHttp: () => vi.fn(),
-  } as unknown as MatrixClient);
-
-  vi.spyOn(client, "mxcUrlToHttp");
-  const member = mockMatrixRoomMember(
-    mockRtcMembership("@alice:example.org", "AAAA"),
-    {
-      getMxcAvatarUrl: () => "mxc://example.org/alice-avatar",
-    },
-  );
-  const displayName = "Alice";
-  render(
-    <TestComponent client={client} supportsThumbnails={false}>
-      <Avatar
-        id={member.userId}
-        name={displayName}
-        size={96}
-        src={member.getMxcAvatarUrl()}
-      />
-    </TestComponent>,
-  );
-  const element = screen.getByRole("img", { name: "@alice:example.org" });
-  expect(element.tagName).toEqual("SPAN");
-  expect(client.mxcUrlToHttp).toBeCalledTimes(0);
-});
-
-test("should attempt to fetch authenticated media", async () => {
+test("should attempt to fetch authenticated media from the server", async () => {
   const expectedAuthUrl = "http://example.org/media/alice-avatar";
   const expectedObjectURL = "my-object-url";
   const accessToken = "my-access-token";
@@ -154,3 +134,47 @@ test("should attempt to fetch authenticated media", async () => {
     headers: { Authorization: `Bearer ${accessToken}` },
   });
 });
+
+test("should attempt to use widget API if running as a widget", async () => {
+  const expectedMXCUrl = "mxc://example.org/alice-avatar";
+  const expectedObjectURL = "my-object-url";
+  const theBlob = new Blob([]);
+
+  // vitest doesn't have a implementation of create/revokeObjectURL, so we need
+  // to delete the property. It's a bit odd, but it works.
+  Reflect.deleteProperty(global.window.URL, "createObjectURL");
+  globalThis.URL.createObjectURL = vi.fn().mockReturnValue(expectedObjectURL);
+  Reflect.deleteProperty(global.window.URL, "revokeObjectURL");
+  globalThis.URL.revokeObjectURL = vi.fn();
+
+  const client = vi.mocked<MatrixClient>({
+    getAccessToken: () => undefined,
+  } as unknown as MatrixClient);
+
+  widget!.api = { downloadFile: vi.fn() } as unknown as WidgetApi;
+  vi.spyOn(widget!.api, "downloadFile").mockResolvedValue({ file: theBlob });
+  const member = mockMatrixRoomMember(
+    mockRtcMembership("@alice:example.org", "AAAA"),
+    {
+      getMxcAvatarUrl: () => expectedMXCUrl,
+    },
+  );
+  const displayName = "Alice";
+  render(
+    <TestComponent client={client}>
+      <Avatar
+        id={member.userId}
+        name={displayName}
+        size={96}
+        src={member.getMxcAvatarUrl()}
+      />
+    </TestComponent>,
+  );
+
+  // Fetch is asynchronous, so wait for this to resolve.
+  await vi.waitUntil(() =>
+    document.querySelector(`img[src='${expectedObjectURL}']`),
+  );
+
+  expect(widget!.api.downloadFile).toBeCalledWith(expectedMXCUrl);
+});

src/Avatar.tsx

@@ -14,8 +14,10 @@ import {
 } from "react";
 import { Avatar as CompoundAvatar } from "@vector-im/compound-web";
 import { type MatrixClient } from "matrix-js-sdk";
+import { type WidgetApi } from "matrix-widget-api";
 
 import { useClientState } from "./ClientContext";
+import { widget } from "./widget";
 
 export enum Size {
   XS = "xs",
@@ -78,50 +80,54 @@ export const Avatar: FC<Props> = ({
   const sizePx = useMemo(
     () =>
       Object.values(Size).includes(size as Size)
-        ? sizes.get(size as Size)
+        ? sizes.get(size as Size)!
         : (size as number),
     [size],
   );
 
   const [avatarUrl, setAvatarUrl] = useState<string | undefined>(undefined);
 
+  // In theory, a change in `clientState` or `sizePx` could run extra getAvatarFromWidgetAPI calls, but in practice they should be stable long before this code runs.
   useEffect(() => {
-    if (clientState?.state !== "valid") {
-      return;
-    }
-    const { authenticated, supportedFeatures } = clientState;
-    const client = authenticated?.client;
-
-    if (!client || !src || !sizePx || !supportedFeatures.thumbnails) {
+    if (!src) {
+      setAvatarUrl(undefined);
       return;
     }
 
-    const token = client.getAccessToken();
-    if (!token) {
-      return;
-    }
-    const resolveSrc = getAvatarUrl(client, src, sizePx);
-    if (!resolveSrc) {
+    let blob: Promise<Blob>;
+
+    if (widget?.api) {
+      blob = getAvatarFromWidgetAPI(widget.api, src);
+    } else if (
+      clientState?.state === "valid" &&
+      clientState.authenticated?.client &&
+      sizePx
+    ) {
+      blob = getAvatarFromServer(clientState.authenticated.client, src, sizePx);
+    } else {
       setAvatarUrl(undefined);
       return;
     }
 
     let objectUrl: string | undefined;
-    fetch(resolveSrc, {
-      headers: {
-        Authorization: `Bearer ${token}`,
-      },
-    })
-      .then(async (req) => req.blob())
+    let stale = false;
+    blob
       .then((blob) => {
+        if (stale) {
+          return;
+        }
         objectUrl = URL.createObjectURL(blob);
         setAvatarUrl(objectUrl);
       })
       .catch((ex) => {
+        if (stale) {
+          return;
+        }
         setAvatarUrl(undefined);
       });
 
     return (): void => {
+      stale = true;
       if (objectUrl) {
         URL.revokeObjectURL(objectUrl);
       }
@@ -140,3 +146,44 @@ export const Avatar: FC<Props> = ({
     />
   );
 };
+
+async function getAvatarFromServer(
+  client: MatrixClient,
+  src: string,
+  sizePx: number,
+): Promise<Blob> {
+  const httpSrc = getAvatarUrl(client, src, sizePx);
+  if (!httpSrc) {
+    throw new Error("Failed to get http avatar URL");
+  }
+
+  const token = client.getAccessToken();
+  if (!token) {
+    throw new Error("Failed to get access token");
+  }
+
+  const request = await fetch(httpSrc, {
+    headers: {
+      Authorization: `Bearer ${token}`,
+    },
+  });
+
+  const blob = await request.blob();
+
+  return blob;
+}
+
+async function getAvatarFromWidgetAPI(
+  api: WidgetApi,
+  src: string,
+): Promise<Blob> {
+  const response = await api.downloadFile(src);
+  const file = response.file;
+
+  // element-web sends a Blob, and the MSC4039 is considering changing the spec to strictly Blob, so only handling that
+  if (!(file instanceof Blob)) {
+    throw new Error("Downloaded file is not a Blob");
+  }
+
+  return file;
+}

src/ClientContext.tsx

@@ -48,7 +48,6 @@ export type ValidClientState = {
   disconnected: boolean;
   supportedFeatures: {
     reactions: boolean;
-    thumbnails: boolean;
   };
   setClient: (client: MatrixClient, session: Session) => void;
 };
@@ -249,7 +248,6 @@ export const ClientProvider: FC<Props> = ({ children }) => {
 
   const [isDisconnected, setIsDisconnected] = useState(false);
   const [supportsReactions, setSupportsReactions] = useState(false);
-  const [supportsThumbnails, setSupportsThumbnails] = useState(false);
 
   const state: ClientState | undefined = useMemo(() => {
     if (alreadyOpenedErr) {
@@ -275,7 +273,6 @@ export const ClientProvider: FC<Props> = ({ children }) => {
       disconnected: isDisconnected,
       supportedFeatures: {
         reactions: supportsReactions,
-        thumbnails: supportsThumbnails,
       },
     };
   }, [
@@ -286,7 +283,6 @@ export const ClientProvider: FC<Props> = ({ children }) => {
     setClient,
     isDisconnected,
     supportsReactions,
-    supportsThumbnails,
   ]);
 
   const onSync = useCallback(
@@ -312,8 +308,6 @@ export const ClientProvider: FC<Props> = ({ children }) => {
     }
 
     if (initClientState.widgetApi) {
-      // There is currently no widget API for authenticated media thumbnails.
-      setSupportsThumbnails(false);
       const reactSend = initClientState.widgetApi.hasCapability(
         "org.matrix.msc2762.send.event:m.reaction",
       );
@@ -335,7 +329,6 @@ export const ClientProvider: FC<Props> = ({ children }) => {
       }
     } else {
       setSupportsReactions(true);
-      setSupportsThumbnails(true);
     }
 
     return (): void => {

src/settings/useSubmitRageshake.test.tsx

@@ -78,7 +78,6 @@ function renderWithMockClient(
         disconnected: false,
         supportedFeatures: {
           reactions: true,
-          thumbnails: true,
         },
         setClient: vi.fn(),
         authenticated: {

src/widget.ts

@@ -93,6 +93,7 @@ export const initializeWidget = (
       logger.info("Widget API is available");
       const api = new WidgetApi(widgetId, parentOrigin);
       api.requestCapability(MatrixCapabilities.AlwaysOnScreen);
+      api.requestCapability(MatrixCapabilities.MSC4039DownloadFile);
 
       // Set up the lazy action emitter, but only for select actions that we
       // intend for the app to handle