Remove most emoji is valid assertions

Co-authored-by: Jamie <113370520+jamiebuilds-signal@users.noreply.github.com>

ts/components/CallScreen.dom.tsx

@@ -88,11 +88,12 @@ import {
   useCallReactionBursts,
 } from './CallReactionBurst.dom.js';
 import { isGroupOrAdhocActiveCall } from '../util/isGroupOrAdhocCall.std.js';
-import { assertDev, strictAssert } from '../util/assert.std.js';
+import { assertDev } from '../util/assert.std.js';
 import { CallingPendingParticipants } from './CallingPendingParticipants.dom.js';
 import type { CallingImageDataCache } from './CallManager.dom.js';
 import { FunStaticEmoji } from './fun/FunEmoji.dom.js';
 import {
+  getEmojiDebugLabel,
   getEmojiParentByKey,
   getEmojiParentKeyByVariantKey,
   getEmojiVariantByKey,
@@ -104,9 +105,12 @@ import {
   BeforeNavigateResponse,
   beforeNavigateService,
 } from '../services/BeforeNavigate.std.js';
+import { createLogger } from '../logging/log.std.js';
 
 const { isEqual, noop } = lodash;
 
+const log = createLogger('CallScreen');
+
 export type PropsType = {
   activeCall: ActiveCallType;
   approveUser: (payload: PendingUserActionPayloadType) => void;
@@ -1300,7 +1304,13 @@ function useReactionsToast(props: UseReactionsToastType): void {
 
       const key = `reactions-${timestamp}-${demuxId}`;
 
-      strictAssert(isEmojiVariantValue(value), 'Expected a valid emoji value');
+      if (!isEmojiVariantValue(value)) {
+        log.error(
+          `Expected a valid emoji value, got ${getEmojiDebugLabel(value)}`
+        );
+        return;
+      }
+
       const emojiVariantKey = getEmojiVariantKeyByValue(value);
       const emojiVariant = getEmojiVariantByKey(emojiVariantKey);
 

ts/components/CompositionInput.dom.tsx

@@ -39,11 +39,7 @@ import type { ConversationType } from '../state/ducks/conversations.preload.js';
 import type { PreferredBadgeSelectorType } from '../state/selectors/badges.preload.js';
 import { isAciString } from '../util/isAciString.std.js';
 import { MentionBlot } from '../quill/mentions/blot.dom.js';
-import {
+import { matchEmojiBlot, matchEmojiText } from '../quill/emoji/matchers.dom.js';
-  matchEmojiImage,
-  matchEmojiBlot,
-  matchEmojiText,
-} from '../quill/emoji/matchers.dom.js';
 import { matchMention } from '../quill/mentions/matchers.std.js';
 import { MemberRepository } from '../quill/memberRepository.std.js';
 import {
@@ -802,8 +798,6 @@ export function CompositionInput(props: Props): React.ReactElement {
                 [Node.TEXT_NODE, matchNewline],
                 ['br', matchBreak],
                 [Node.ELEMENT_NODE, matchNewline],
-                ['IMG', matchEmojiImage],
-                ['SPAN', matchEmojiImage],
                 ['IMG', matchEmojiBlot],
                 ['SPAN', matchEmojiBlot],
                 ['STRONG', matchBold],

ts/components/ReactionPickerPicker.dom.tsx

@@ -8,12 +8,15 @@ import classNames from 'classnames';
 import { Button } from 'react-aria-components';
 import type { LocalizerType } from '../types/Util.std.js';
 import { FunStaticEmoji } from './fun/FunEmoji.dom.js';
-import { strictAssert } from '../util/assert.std.js';
 import {
+  getEmojiDebugLabel,
   getEmojiVariantByKey,
   getEmojiVariantKeyByValue,
   isEmojiVariantValue,
 } from './fun/data/emojis.std.js';
+import { createLogger } from '../logging/log.std.js';
+
+const log = createLogger('ReactionPickerPicker');
 
 export enum ReactionPickerPickerStyle {
   Picker,
@@ -32,10 +35,13 @@ export const ReactionPickerPickerEmojiButton = React.forwardRef<
   { emoji, onClick, isSelected, title },
   ref
 ) {
-  strictAssert(
+  if (!isEmojiVariantValue(emoji)) {
-    isEmojiVariantValue(emoji),
+    log.error(
-    'Expected a valid emoji variant value'
+      `Expected a valid emoji variant value, got ${getEmojiDebugLabel(emoji)}`
-  );
+    );
+    return null;
+  }
+
   const emojiVariantKey = getEmojiVariantKeyByValue(emoji);
   const emojiVariant = getEmojiVariantByKey(emojiVariantKey);
 

ts/components/conversation/Message.dom.tsx

@@ -108,7 +108,7 @@ import { getColorForCallLink } from '../../util/getColorForCallLink.std.js';
 import { getKeyFromCallLink } from '../../util/callLinks.std.js';
 import { InAnotherCallTooltip } from './InAnotherCallTooltip.dom.js';
 import { formatFileSize } from '../../util/formatFileSize.std.js';
-import { assertDev, strictAssert } from '../../util/assert.std.js';
+import { assertDev } from '../../util/assert.std.js';
 import { AttachmentStatusIcon } from './AttachmentStatusIcon.dom.js';
 import { TapToViewNotAvailableType } from '../TapToViewNotAvailableModal.dom.js';
 import type { DataPropsType as TapToViewNotAvailablePropsType } from '../TapToViewNotAvailableModal.dom.js';
@@ -116,6 +116,7 @@ import { FileThumbnail } from '../FileThumbnail.dom.js';
 import { FunStaticEmoji } from '../fun/FunEmoji.dom.js';
 import {
   type EmojifyData,
+  getEmojiDebugLabel,
   getEmojifyData,
   getEmojiParentByKey,
   getEmojiParentKeyByVariantKey,
@@ -219,10 +220,13 @@ export type GiftBadgeType =
     };
 
 function ReactionEmoji(props: { emojiVariantValue: string }) {
-  strictAssert(
+  if (!isEmojiVariantValue(props.emojiVariantValue)) {
-    isEmojiVariantValue(props.emojiVariantValue),
+    log.error(
-    'Expected a valid emoji variant value'
+      `Expected a valid emoji variant value, got ${getEmojiDebugLabel(props.emojiVariantValue)}`
-  );
+    );
+    return null;
+  }
+
   const emojiVariantKey = getEmojiVariantKeyByValue(props.emojiVariantValue);
   const emojiVariant = getEmojiVariantByKey(emojiVariantKey);
   const emojiParentKey = getEmojiParentKeyByVariantKey(emojiVariantKey);

ts/components/conversation/ReactionViewer.dom.tsx

@@ -18,6 +18,7 @@ import type {
 } from '../fun/data/emojis.std.js';
 import {
   EMOJI_PARENT_KEY_CONSTANTS,
+  getEmojiDebugLabel,
   getEmojiParentKeyByVariantKey,
   getEmojiVariantByKey,
   getEmojiVariantKeyByValue,
@@ -26,9 +27,12 @@ import {
 import { strictAssert } from '../../util/assert.std.js';
 import { FunStaticEmoji } from '../fun/FunEmoji.dom.js';
 import { useFunEmojiLocalizer } from '../fun/useFunEmojiLocalizer.dom.js';
+import { createLogger } from '../../logging/log.std.js';
 
 const { mapValues, orderBy } = lodash;
 
+const log = createLogger('ReactionViewer');
+
 export type Reaction = {
   emoji: string;
   timestamp: number;
@@ -84,13 +88,17 @@ type ReactionWithEmojiData = Reaction &
 
 function ReactionViewerEmoji(props: {
   emojiVariantValue: string | undefined;
-}): JSX.Element {
+}): JSX.Element | null {
   const emojiLocalizer = useFunEmojiLocalizer();
   strictAssert(props.emojiVariantValue != null, 'Expected an emoji');
-  strictAssert(
+
-    isEmojiVariantValue(props.emojiVariantValue),
+  if (!isEmojiVariantValue(props.emojiVariantValue)) {
-    'Must be valid emoji variant value'
+    log.error(
-  );
+      `Must be valid emoji variant value, got ${getEmojiDebugLabel(props.emojiVariantValue)}`
+    );
+    return null;
+  }
+
   const emojiVariantKey = getEmojiVariantKeyByValue(props.emojiVariantValue);
   const emojiVariant = getEmojiVariantByKey(emojiVariantKey);
   return (

ts/components/fun/FunEmoji.dom.tsx

@@ -4,8 +4,16 @@ import classNames from 'classnames';
 import type { CSSProperties } from 'react';
 import React, { useMemo } from 'react';
 import MANIFEST from '../../../build/jumbomoji.json';
-import type { EmojiVariantData } from './data/emojis.std.js';
+import {
+  getEmojiDebugLabel,
+  isSafeEmojifyEmoji,
+  type EmojiVariantData,
+  type EmojiVariantValue,
+} from './data/emojis.std.js';
 import type { FunImageAriaProps } from './types.dom.js';
+import { createLogger } from '../../logging/log.std.js';
+
+const log = createLogger('FunEmoji');
 
 export const FUN_STATIC_EMOJI_CLASS = 'FunStaticEmoji';
 export const FUN_INLINE_EMOJI_CLASS = 'FunInlineEmoji';
@@ -113,13 +121,13 @@ const TRANSPARENT_PIXEL =
  * We need to use the `<img>` bec ause
  */
 export function createStaticEmojiBlot(
-  node: HTMLImageElement,
+  nodeParam: HTMLImageElement,
   props: StaticEmojiBlotProps
 ): void {
+  const node = nodeParam;
+
   const jumboImage = getEmojiJumboBackground(props.emoji, props.size);
-  // eslint-disable-next-line no-param-reassign
   node.src = TRANSPARENT_PIXEL;
-  // eslint-disable-next-line no-param-reassign
   node.role = props.role;
   node.classList.add(FUN_STATIC_EMOJI_CLASS);
   if (jumboImage != null) {
@@ -133,6 +141,10 @@ export function createStaticEmojiBlot(
   node.style.setProperty('--fun-emoji-sheet-x', `${props.emoji.sheetX}`);
   node.style.setProperty('--fun-emoji-sheet-y', `${props.emoji.sheetY}`);
   node.style.setProperty('--fun-emoji-jumbo-image', jumboImage);
+
+  // Needed to lookup emoji value in `matchEmojiBlot`
+  node.dataset.emojiKey = props.emoji.key;
+  node.dataset.emojiValue = props.emoji.value;
 }
 
 export type FunInlineEmojiProps = FunImageAriaProps &
@@ -154,6 +166,7 @@ export function FunInlineEmoji(props: FunInlineEmojiProps): JSX.Element {
       width={64}
       height={64}
       viewBox="0 0 64 64"
+      // Needed to lookup emoji value in `matchEmojiBlot`
       data-emoji-key={props.emoji.key}
       data-emoji-value={props.emoji.value}
       style={
@@ -192,3 +205,33 @@ export function FunInlineEmoji(props: FunInlineEmojiProps): JSX.Element {
     </svg>
   );
 }
+
+export function isFunEmojiElement(element: HTMLElement): boolean {
+  return (
+    element.classList.contains(FUN_INLINE_EMOJI_CLASS) ||
+    element.classList.contains(FUN_STATIC_EMOJI_CLASS)
+  );
+}
+
+export function getFunEmojiElementValue(
+  element: HTMLElement
+): EmojiVariantValue | null {
+  if (!isFunEmojiElement(element)) {
+    return null;
+  }
+
+  const value = element.dataset.emojiValue;
+  if (value == null) {
+    log.error('Missing a data-emoji-value attribute on emoji element');
+    return null;
+  }
+
+  if (!isSafeEmojifyEmoji(value)) {
+    log.error(
+      `Expected a valid emoji variant value, got ${getEmojiDebugLabel(value)}`
+    );
+    return null;
+  }
+
+  return value;
+}

ts/components/fun/data/emojis.std.ts

@@ -10,6 +10,9 @@ import type {
 } from '../useFunEmojiSearch.dom.js';
 import type { FunEmojiLocalizerIndex } from '../useFunEmojiLocalizer.dom.js';
 import { removeDiacritics } from '../../../util/removeDiacritics.std.js';
+import { createLogger } from '../../../logging/log.std.js';
+
+const log = createLogger('fun/data/emojis');
 
 // Import emoji-datasource dynamically to avoid costly typechecking.
 // eslint-disable-next-line import/no-dynamic-require, @typescript-eslint/no-var-requires
@@ -485,6 +488,14 @@ for (const rawEmoji of RAW_EMOJI_DATA) {
   addParent(parent, rawEmoji.sort_order);
 }
 
+export function getEmojiDebugLabel(input: string): string {
+  return Array.from(input.slice(0, 12), char => {
+    const num = char.codePointAt(0) ?? 0;
+    const hex = num.toString(16).toUpperCase().padStart(4, '0');
+    return `U+${hex}`;
+  }).join(' ');
+}
+
 export function isEmojiParentKey(input: string): input is EmojiParentKey {
   return EMOJI_INDEX.parentByKey.has(input as EmojiParentKey);
 }
@@ -731,7 +742,11 @@ export function getEmojifyData(input: string): EmojifyData {
     const value = match[0];
 
     // Only consider safe emojis as matches
-    if (isSafeEmojifyEmoji(value)) {
+    if (!isSafeEmojifyEmoji(value)) {
+      log.warn(
+        `Expected a valid emoji variant value, got ${getEmojiDebugLabel(value)}`
+      );
+    } else {
       const { index } = match;
       hasEmojis = true;
       // Track if we skipped over any text

ts/quill/emoji/blot.dom.tsx

@@ -2,22 +2,22 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import EmbedBlot from '@signalapp/quill-cjs/blots/embed.js';
-import { strictAssert } from '../../util/assert.std.js';
+import type { EmojiVariantValue } from '../../components/fun/data/emojis.std.js';
 import {
   getEmojiVariantByKey,
   getEmojiVariantKeyByValue,
-  isEmojiVariantValue,
 } from '../../components/fun/data/emojis.std.js';
 import {
   createStaticEmojiBlot,
   FUN_STATIC_EMOJI_CLASS,
+  getFunEmojiElementValue,
 } from '../../components/fun/FunEmoji.dom.js';
 
 // the DOM structure of this EmojiBlot should match the other emoji implementations:
 // ts/components/fun/FunEmoji.tsx
 
 export type EmojiBlotValue = Readonly<{
-  value: string;
+  value: EmojiVariantValue;
   source?: string;
 }>;
 
@@ -32,7 +32,6 @@ export class EmojiBlot extends EmbedBlot {
   static override create({ value: emoji, source }: EmojiBlotValue): Node {
     const node = super.create(undefined) as HTMLImageElement;
 
-    strictAssert(isEmojiVariantValue(emoji), 'Value is not a known emoji');
     const variantKey = getEmojiVariantKeyByValue(emoji);
     const variant = getEmojiVariantByKey(variantKey);
 
@@ -42,16 +41,18 @@ export class EmojiBlot extends EmbedBlot {
       emoji: variant,
       size: 20,
     });
-    node.setAttribute('data-emoji', emoji);
+    node.setAttribute('data-emoji-key', variantKey);
-    node.setAttribute('data-emoji', emoji);
+    node.setAttribute('data-emoji-value', emoji);
     node.setAttribute('data-source', source ?? '');
 
     return node;
   }
 
   static override value(node: HTMLElement): EmojiBlotValue | undefined {
-    const { emoji, source } = node.dataset;
+    const emoji = getFunEmojiElementValue(node);
-    if (emoji === undefined) {
+    const { source } = node.dataset;
+
+    if (emoji == null) {
       throw new Error(
         `Failed to make EmojiBlot with emoji: ${emoji}, source: ${source}`
       );

ts/quill/emoji/matchers.dom.ts

@@ -5,27 +5,7 @@ import { Delta } from '@signalapp/quill-cjs';
 
 import { insertEmojiOps } from '../util.dom.js';
 import type { Matcher } from '../util.dom.js';
-import {
+import { getFunEmojiElementValue } from '../../components/fun/FunEmoji.dom.js';
-  FUN_INLINE_EMOJI_CLASS,
-  FUN_STATIC_EMOJI_CLASS,
-} from '../../components/fun/FunEmoji.dom.js';
-
-export const matchEmojiImage: Matcher = (
-  node,
-  delta,
-  _scroll,
-  attributes
-): Delta => {
-  if (
-    node.classList.contains(FUN_INLINE_EMOJI_CLASS) ||
-    (node.classList.contains(FUN_STATIC_EMOJI_CLASS) &&
-      node.dataset.emoji == null)
-  ) {
-    const value = node.getAttribute('aria-label');
-    return new Delta().insert({ emoji: { value } }, attributes);
-  }
-  return delta;
-};
 
 export const matchEmojiBlot: Matcher = (
   node,
@@ -33,11 +13,9 @@ export const matchEmojiBlot: Matcher = (
   _scroll,
   attributes
 ): Delta => {
-  if (
+  const value = getFunEmojiElementValue(node);
-    node.classList.contains(FUN_STATIC_EMOJI_CLASS) &&
+  if (value != null) {
-    node.dataset.emoji != null
+    const { source } = node.dataset;
-  ) {
-    const { emoji: value, source } = node.dataset;
     return new Delta().insert({ emoji: { value, source } }, attributes);
   }
   return delta;

ts/quill/signal-clipboard/util.dom.ts

@@ -1,10 +1,7 @@
 // Copyright 2023 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import {
+import { getFunEmojiElementValue } from '../../components/fun/FunEmoji.dom.js';
-  FUN_INLINE_EMOJI_CLASS,
-  FUN_STATIC_EMOJI_CLASS,
-} from '../../components/fun/FunEmoji.dom.js';
 
 const QUILL_EMBED_GUARD = '\uFEFF';
 
@@ -61,6 +58,10 @@ export function createEventHandler({
   };
 }
 
+function isHTMLElement(node: Node): node is HTMLElement {
+  return node.nodeType === Node.ELEMENT_NODE;
+}
+
 function getStringFromNode(
   node: Node,
   parent?: Node,
@@ -72,20 +73,14 @@ function getStringFromNode(
     }
     return node.textContent || '';
   }
-  if (node.nodeType !== Node.ELEMENT_NODE) {
+  if (!isHTMLElement(node)) {
     return '';
   }
+  const element = node;
 
-  const element = node as Element;
+  const emojiValue = getFunEmojiElementValue(element);
-  if (
+  if (emojiValue != null) {
-    element.classList.contains(FUN_STATIC_EMOJI_CLASS) ||
+    return emojiValue;
-    element.classList.contains(FUN_INLINE_EMOJI_CLASS)
-  ) {
-    return (
-      element.ariaLabel ||
-      element.attributes.getNamedItem('data-emoji-value')?.value ||
-      ''
-    );
   }
 
   // Sometimes we need to add multiple newlines to represent nested divs, and other times

ts/quill/util.dom.ts

@@ -20,10 +20,14 @@ import {
 import { isNotNil } from '../util/isNotNil.std.js';
 import type { AciString } from '../types/ServiceId.std.js';
 import {
+  getEmojiDebugLabel,
   getEmojiVariantByKey,
   getEmojiVariantKeyByValue,
   isSafeEmojifyEmoji,
 } from '../components/fun/data/emojis.std.js';
+import { createLogger } from '../logging/log.std.js';
+
+const log = createLogger('quill/util');
 
 export type Matcher = (
   node: HTMLElement,
@@ -466,17 +470,21 @@ export const insertEmojiOps = (
       // eslint-disable-next-line no-cond-assign
       while ((match = re.exec(text))) {
         const [emojiMatch] = match;
-        if (isSafeEmojifyEmoji(emojiMatch)) {
+        if (!isSafeEmojifyEmoji(emojiMatch)) {
-          const variantKey = getEmojiVariantKeyByValue(emojiMatch);
+          log.error(
-          const variant = getEmojiVariantByKey(variantKey);
+            `Expected a valid emoji variant value, got ${getEmojiDebugLabel(emojiMatch)}`
-
+          );
-          ops.push({ insert: text.slice(index, match.index), attributes });
+          continue;
-          ops.push({
-            insert: { emoji: { value: variant.value } },
-            attributes: { ...existingAttributes, ...attributes },
-          });
-          index = match.index + variant.value.length;
         }
+        const variantKey = getEmojiVariantKeyByValue(emojiMatch);
+        const variant = getEmojiVariantByKey(variantKey);
+
+        ops.push({ insert: text.slice(index, match.index), attributes });
+        ops.push({
+          insert: { emoji: { value: variant.value } },
+          attributes: { ...existingAttributes, ...attributes },
+        });
+        index = match.index + variant.value.length;
       }
 
       ops.push({ insert: text.slice(index, text.length), attributes });