Member avatars without canvas (#9990)

* Strict typechecking fixes for Base/Member/Avatar

Update the core avatar files to pass `--strict --noImplicitAny` typechecks.

Signed-off-by: Clark Fischer <[email protected]>

* Add tests for Base/Member/Avatar

More thoroughly test the core avatar files. Not necessarily the most thorough,
but an improvement.

Signed-off-by: Clark Fischer <[email protected]>

* Extract TextAvatar from BaseAvatar

Extracted the fallback/textual avatar into its own component.

Signed-off-by: Clark Fischer <[email protected]>

* Use standard HTML for non-image avatars

Firefox users with `resistFingerprinting` enabled were seeing random noise
for rooms and users without avatars. There's no real reason to use data
URLs to present flat colors.

This converts non-image avatars to inline blocks with background colors.

See element-hq/element-web#23936

Signed-off-by: Clark Fischer <[email protected]>

* Have pills use solid backgrounds rather than colored images

Similar to room and member avatars, pills now use colored pseudo-elements
rather than background images.

Signed-off-by: Clark Fischer <[email protected]>

---------

Signed-off-by: Clark Fischer <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>

Author: clarkf

res/css/views/rooms/_BasicMessageComposer.pcss

@@ -78,7 +78,7 @@ limitations under the License.
                     min-width: $font-16px; /* ensure the avatar is not compressed */
                     height: $font-16px;
                     margin-inline-end: 0.24rem;
-                    background: var(--avatar-background), $background;
+                    background: var(--avatar-background);
                     color: $avatar-initial-color;
                     background-repeat: no-repeat;
                     background-size: $font-16px;

src/Avatar.ts

@@ -1,5 +1,5 @@
 /*
-Copyright 2015, 2016 OpenMarket Ltd
+Copyright 2015, 2016, 2023 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -24,16 +24,19 @@ import DMRoomMap from "./utils/DMRoomMap";
 import { mediaFromMxc } from "./customisations/Media";
 import { isLocalRoom } from "./utils/localRoom/isLocalRoom";
 
+const DEFAULT_COLORS: Readonly<string[]> = ["#0DBD8B", "#368bd6", "#ac3ba8"];
+
 // Not to be used for BaseAvatar urls as that has similar default avatar fallback already
 export function avatarUrlForMember(
-    member: RoomMember,
+    member: RoomMember | null | undefined,
     width: number,
     height: number,
     resizeMethod: ResizeMethod,
 ): string {
-    let url: string;
-    if (member?.getMxcAvatarUrl()) {
-        url = mediaFromMxc(member.getMxcAvatarUrl()).getThumbnailOfSourceHttp(width, height, resizeMethod);
+    let url: string | undefined;
+    const mxcUrl = member?.getMxcAvatarUrl();
+    if (mxcUrl) {
+        url = mediaFromMxc(mxcUrl).getThumbnailOfSourceHttp(width, height, resizeMethod);
     }
     if (!url) {
         // member can be null here currently since on invites, the JS SDK
@@ -44,6 +47,17 @@ export function avatarUrlForMember(
     return url;
 }
 
+export function getMemberAvatar(
+    member: RoomMember | null | undefined,
+    width: number,
+    height: number,
+    resizeMethod: ResizeMethod,
+): string | undefined {
+    const mxcUrl = member?.getMxcAvatarUrl();
+    if (!mxcUrl) return undefined;
+    return mediaFromMxc(mxcUrl).getThumbnailOfSourceHttp(width, height, resizeMethod);
+}
+
 export function avatarUrlForUser(
     user: Pick<User, "avatarUrl">,
     width: number,
@@ -86,18 +100,10 @@ function urlForColor(color: string): string {
 // hard to install a listener here, even if there were a clear event to listen to
 const colorToDataURLCache = new Map<string, string>();
 
-export function defaultAvatarUrlForString(s: string): string {
+export function defaultAvatarUrlForString(s: string | undefined): string {
     if (!s) return ""; // XXX: should never happen but empirically does by evidence of a rageshake
-    const defaultColors = ["#0DBD8B", "#368bd6", "#ac3ba8"];
-    let total = 0;
-    for (let i = 0; i < s.length; ++i) {
-        total += s.charCodeAt(i);
-    }
-    const colorIndex = total % defaultColors.length;
-    // overwritten color value in custom themes
-    const cssVariable = `--avatar-background-colors_${colorIndex}`;
-    const cssValue = document.body.style.getPropertyValue(cssVariable);
-    const color = cssValue || defaultColors[colorIndex];
+
+    const color = getColorForString(s);
     let dataUrl = colorToDataURLCache.get(color);
     if (!dataUrl) {
         // validate color as this can come from account_data
@@ -112,13 +118,23 @@ export function defaultAvatarUrlForString(s: string): string {
     return dataUrl;
 }
 
+export function getColorForString(input: string): string {
+    const charSum = [...input].reduce((s, c) => s + c.charCodeAt(0), 0);
+    const index = charSum % DEFAULT_COLORS.length;
+
+    // overwritten color value in custom themes
+    const cssVariable = `--avatar-background-colors_${index}`;
+    const cssValue = document.body.style.getPropertyValue(cssVariable);
+    return cssValue || DEFAULT_COLORS[index]!;
+}
+
 /**
  * returns the first (non-sigil) character of 'name',
  * converted to uppercase
  * @param {string} name
  * @return {string} the first letter
  */
-export function getInitialLetter(name: string): string {
+export function getInitialLetter(name: string): string | undefined {
     if (!name) {
         // XXX: We should find out what causes the name to sometimes be falsy.
         console.trace("`name` argument to `getInitialLetter` not supplied");
@@ -134,19 +150,20 @@ export function getInitialLetter(name: string): string {
     }
 
     // rely on the grapheme cluster splitter in lodash so that we don't break apart compound emojis
-    return split(name, "", 1)[0].toUpperCase();
+    return split(name, "", 1)[0]!.toUpperCase();
 }
 
 export function avatarUrlForRoom(
-    room: Room,
+    room: Room | undefined,
     width: number,
     height: number,
     resizeMethod?: ResizeMethod,
 ): string | null {
     if (!room) return null; // null-guard
 
-    if (room.getMxcAvatarUrl()) {
-        return mediaFromMxc(room.getMxcAvatarUrl()).getThumbnailOfSourceHttp(width, height, resizeMethod);
+    const mxcUrl = room.getMxcAvatarUrl();
+    if (mxcUrl) {
+        return mediaFromMxc(mxcUrl).getThumbnailOfSourceHttp(width, height, resizeMethod);
     }
 
     // space rooms cannot be DMs so skip the rest
@@ -159,8 +176,9 @@ export function avatarUrlForRoom(
 
     // If there are only two members in the DM use the avatar of the other member
     const otherMember = room.getAvatarFallbackMember();
-    if (otherMember?.getMxcAvatarUrl()) {
-        return mediaFromMxc(otherMember.getMxcAvatarUrl()).getThumbnailOfSourceHttp(width, height, resizeMethod);
+    const otherMemberMxc = otherMember?.getMxcAvatarUrl();
+    if (otherMemberMxc) {
+        return mediaFromMxc(otherMemberMxc).getThumbnailOfSourceHttp(width, height, resizeMethod);
     }
     return null;
 }

src/components/views/avatars/BaseAvatar.tsx

@@ -1,8 +1,6 @@
 /*
-Copyright 2015, 2016 OpenMarket Ltd
-Copyright 2018 New Vector Ltd
 Copyright 2019 Michael Telatynski <[email protected]>
-Copyright 2019, 2020 The Matrix.org Foundation C.I.C.
+Copyright 2015, 2016, 2018, 2019, 2020, 2023 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -21,34 +19,41 @@ import React, { useCallback, useContext, useEffect, useState } from "react";
 import classNames from "classnames";
 import { ResizeMethod } from "matrix-js-sdk/src/@types/partials";
 import { ClientEvent } from "matrix-js-sdk/src/client";
+import { SyncState } from "matrix-js-sdk/src/sync";
 
 import * as AvatarLogic from "../../../Avatar";
-import SettingsStore from "../../../settings/SettingsStore";
 import AccessibleButton from "../elements/AccessibleButton";
 import RoomContext from "../../../contexts/RoomContext";
 import MatrixClientContext from "../../../contexts/MatrixClientContext";
 import { useTypedEventEmitter } from "../../../hooks/useEventEmitter";
 import { toPx } from "../../../utils/units";
 import { _t } from "../../../languageHandler";
+import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
 
 interface IProps {
-    name: string; // The name (first initial used as default)
-    idName?: string; // ID for generating hash colours
-    title?: string; // onHover title text
-    url?: string; // highest priority of them all, shortcut to set in urls[0]
-    urls?: string[]; // [highest_priority, ... , lowest_priority]
+    /** The name (first initial used as default) */
+    name: string;
+    /** ID for generating hash colours */
+    idName?: string;
+    /** onHover title text */
+    title?: string;
+    /** highest priority of them all, shortcut to set in urls[0] */
+    url?: string;
+    /** [highest_priority, ... , lowest_priority] */
+    urls?: string[];
     width?: number;
     height?: number;
-    // XXX: resizeMethod not actually used.
+    /** @deprecated not actually used */
     resizeMethod?: ResizeMethod;
-    defaultToInitialLetter?: boolean; // true to add default url
-    onClick?: React.MouseEventHandler;
+    /** true to add default url */
+    defaultToInitialLetter?: boolean;
+    onClick?: React.ComponentPropsWithoutRef<typeof AccessibleTooltipButton>["onClick"];
     inputRef?: React.RefObject<HTMLImageElement & HTMLSpanElement>;
     className?: string;
     tabIndex?: number;
 }
 
-const calculateUrls = (url: string, urls: string[], lowBandwidth: boolean): string[] => {
+const calculateUrls = (url: string | undefined, urls: string[] | undefined, lowBandwidth: boolean): string[] => {
     // work out the full set of urls to try to load. This is formed like so:
     // imageUrls: [ props.url, ...props.urls ]
 
@@ -66,11 +71,26 @@ const calculateUrls = (url: string, urls: string[], lowBandwidth: boolean): stri
     return Array.from(new Set(_urls));
 };
 
-const useImageUrl = ({ url, urls }): [string, () => void] => {
+/**
+ * Hook for cycling through a changing set of images.
+ *
+ * The set of images is updated whenever `url` or `urls` change, the user's
+ * `lowBandwidth` preference changes, or the client reconnects.
+ *
+ * Returns `[imageUrl, onError]`. When `onError` is called, the next image in
+ * the set will be displayed.
+ */
+const useImageUrl = ({
+    url,
+    urls,
+}: {
+    url: string | undefined;
+    urls: string[] | undefined;
+}): [string | undefined, () => void] => {
     // Since this is a hot code path and the settings store can be slow, we
     // use the cached lowBandwidth value from the room context if it exists
     const roomContext = useContext(RoomContext);
-    const lowBandwidth = roomContext ? roomContext.lowBandwidth : SettingsStore.getValue("lowBandwidth");
+    const lowBandwidth = roomContext.lowBandwidth;
 
     const [imageUrls, setUrls] = useState<string[]>(calculateUrls(url, urls, lowBandwidth));
     const [urlsIndex, setIndex] = useState<number>(0);
@@ -85,10 +105,10 @@ const useImageUrl = ({ url, urls }): [string, () => void] => {
     }, [url, JSON.stringify(urls)]); // eslint-disable-line react-hooks/exhaustive-deps
 
     const cli = useContext(MatrixClientContext);
-    const onClientSync = useCallback((syncState, prevState) => {
+    const onClientSync = useCallback((syncState: SyncState, prevState: SyncState | null) => {
         // Consider the client reconnected if there is no error with syncing.
         // This means the state could be RECONNECTING, SYNCING, PREPARED or CATCHUP.
-        const reconnected = syncState !== "ERROR" && prevState !== syncState;
+        const reconnected = syncState !== SyncState.Error && prevState !== syncState;
         if (reconnected) {
             setIndex(0);
         }
@@ -108,46 +128,18 @@ const BaseAvatar: React.FC<IProps> = (props) => {
         urls,
         width = 40,
         height = 40,
-        resizeMethod = "crop", // eslint-disable-line @typescript-eslint/no-unused-vars
         defaultToInitialLetter = true,
         onClick,
         inputRef,
         className,
+        resizeMethod: _unused, // to keep it from being in `otherProps`
         ...otherProps
     } = props;
 
     const [imageUrl, onError] = useImageUrl({ url, urls });
 
     if (!imageUrl && defaultToInitialLetter && name) {
-        const initialLetter = AvatarLogic.getInitialLetter(name);
-        const textNode = (
-            <span
-                className="mx_BaseAvatar_initial"
-                aria-hidden="true"
-                style={{
-                    fontSize: toPx(width * 0.65),
-                    width: toPx(width),
-                    lineHeight: toPx(height),
-                }}
-            >
-                {initialLetter}
-            </span>
-        );
-        const imgNode = (
-            <img
-                className="mx_BaseAvatar_image"
-                src={AvatarLogic.defaultAvatarUrlForString(idName || name)}
-                alt=""
-                title={title}
-                onError={onError}
-                style={{
-                    width: toPx(width),
-                    height: toPx(height),
-                }}
-                aria-hidden="true"
-                data-testid="avatar-img"
-            />
-        );
+        const avatar = <TextAvatar name={name} idName={idName} width={width} height={height} title={title} />;
 
         if (onClick) {
             return (
@@ -159,9 +151,12 @@ const BaseAvatar: React.FC<IProps> = (props) => {
                     className={classNames("mx_BaseAvatar", className)}
                     onClick={onClick}
                     inputRef={inputRef}
+                    style={{
+                        width: toPx(width),
+                        height: toPx(height),
+                    }}
                 >
-                    {textNode}
-                    {imgNode}
+                    {avatar}
                 </AccessibleButton>
             );
         } else {
@@ -170,10 +165,13 @@ const BaseAvatar: React.FC<IProps> = (props) => {
                     className={classNames("mx_BaseAvatar", className)}
                     ref={inputRef}
                     {...otherProps}
+                    style={{
+                        width: toPx(width),
+                        height: toPx(height),
+                    }}
                     role="presentation"
                 >
-                    {textNode}
-                    {imgNode}
+                    {avatar}
                 </span>
             );
         }
@@ -220,3 +218,31 @@ const BaseAvatar: React.FC<IProps> = (props) => {
 
 export default BaseAvatar;
 export type BaseAvatarType = React.FC<IProps>;
+
+const TextAvatar: React.FC<{
+    name: string;
+    idName?: string;
+    width: number;
+    height: number;
+    title?: string;
+}> = ({ name, idName, width, height, title }) => {
+    const initialLetter = AvatarLogic.getInitialLetter(name);
+
+    return (
+        <span
+            className="mx_BaseAvatar_image mx_BaseAvatar_initial"
+            aria-hidden="true"
+            style={{
+                backgroundColor: AvatarLogic.getColorForString(idName || name),
+                width: toPx(width),
+                height: toPx(height),
+                fontSize: toPx(width * 0.65),
+                lineHeight: toPx(height),
+            }}
+            title={title}
+            data-testid="avatar-img"
+        >
+            {initialLetter}
+        </span>
+    );
+};