Link previews: show full size image less often

ts/components/conversation/Message.tsx

@@ -26,6 +26,8 @@ import {
 import { Props as ReactionPickerProps } from './ReactionPicker';
 import { Emoji } from '../emoji/Emoji';
 import { LinkPreviewDate } from './LinkPreviewDate';
+import { LinkPreviewType } from '../../types/message/LinkPreviews';
+import { shouldUseFullSizeLinkPreviewImage } from '../../linkPreviews/shouldUseFullSizeLinkPreviewImage';
 
 import {
   AttachmentType,
@@ -54,22 +56,10 @@ interface Trigger {
   handleContextClick: (event: React.MouseEvent<HTMLDivElement>) => void;
 }
 
-// Same as MIN_WIDTH in ImageGrid.tsx
-const MINIMUM_LINK_PREVIEW_IMAGE_WIDTH = 200;
 const STICKER_SIZE = 200;
 const SELECTED_TIMEOUT = 1000;
 const THREE_HOURS = 3 * 60 * 60 * 1000;
 
-interface LinkPreviewType {
-  title: string;
-  description?: string;
-  domain: string;
-  url: string;
-  isStickerPack: boolean;
-  image?: AttachmentType;
-  date?: number;
-}
-
 export const MessageStatuses = [
   'delivered',
   'error',
@@ -850,12 +840,8 @@ export class Message extends React.PureComponent<Props, State> {
       Boolean(quote) ||
       (conversationType === 'group' && direction === 'incoming');
 
-    const previewHasImage = first.image && isImageAttachment(first.image);
+    const previewHasImage = isImageAttachment(first.image);
-    const width = first.image && first.image.width;
+    const isFullSizeImage = shouldUseFullSizeLinkPreviewImage(first);
-    const isFullSizeImage =
-      !first.isStickerPack &&
-      width &&
-      width >= MINIMUM_LINK_PREVIEW_IMAGE_WIDTH;
 
     const linkPreviewDate = first.date || null;
 
@@ -1498,27 +1484,18 @@ export class Message extends React.PureComponent<Props, State> {
       }
     }
 
-    if (previews && previews.length) {
+    const firstLinkPreview = (previews || [])[0];
-      const first = previews[0];
-
-      if (!first || !first.image) {
-        return undefined;
-      }
-      const { width } = first.image;
-
     if (
-        !first.isStickerPack &&
+      firstLinkPreview &&
-        isImageAttachment(first.image) &&
+      firstLinkPreview.image &&
-        width &&
+      shouldUseFullSizeLinkPreviewImage(firstLinkPreview)
-        width >= MINIMUM_LINK_PREVIEW_IMAGE_WIDTH
     ) {
-        const dimensions = getImageDimensions(first.image);
+      const dimensions = getImageDimensions(firstLinkPreview.image);
       if (dimensions) {
         // Add two for 1px border
         return dimensions.width + 2;
       }
     }
-    }
 
     return undefined;
   }
@@ -1547,10 +1524,6 @@ export class Message extends React.PureComponent<Props, State> {
       const first = previews[0];
       const { image } = first;
 
-      if (!image) {
-        return false;
-      }
-
       return isImageAttachment(image);
     }
 

ts/components/conversation/StagedLinkPreview.tsx

@@ -32,7 +32,7 @@ export const StagedLinkPreview: React.FC<Props> = ({
   date,
   domain,
 }: Props) => {
-  const isImage = image && isImageAttachment(image);
+  const isImage = isImageAttachment(image);
 
   return (
     <div

ts/linkPreviews/shouldUseFullSizeLinkPreviewImage.ts

@@ -0,0 +1,34 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { LinkPreviewType } from '../types/message/LinkPreviews';
+import { isImageAttachment } from '../types/Attachment';
+
+const MINIMUM_FULL_SIZE_DIMENSION = 200;
+
+export function shouldUseFullSizeLinkPreviewImage({
+  isStickerPack,
+  image,
+}: Readonly<LinkPreviewType>): boolean {
+  if (isStickerPack || !isImageAttachment(image)) {
+    return false;
+  }
+
+  const { width, height } = image;
+
+  return (
+    isDimensionFullSize(width) &&
+    isDimensionFullSize(height) &&
+    !isRoughlySquare(width, height)
+  );
+}
+
+function isDimensionFullSize(dimension: unknown): dimension is number {
+  return (
+    typeof dimension === 'number' && dimension >= MINIMUM_FULL_SIZE_DIMENSION
+  );
+}
+
+function isRoughlySquare(width: number, height: number): boolean {
+  return Math.abs(1 - width / height) < 0.05;
+}

ts/test-both/linkPreviews/shouldUseFullSizeLinkPreviewImage_test.ts

@@ -0,0 +1,146 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import { IMAGE_JPEG, VIDEO_MP4 } from '../../types/MIME';
+import { AttachmentType } from '../../types/Attachment';
+
+import { shouldUseFullSizeLinkPreviewImage } from '../../linkPreviews/shouldUseFullSizeLinkPreviewImage';
+
+describe('shouldUseFullSizeLinkPreviewImage', () => {
+  const baseLinkPreview = {
+    title: 'Foo Bar',
+    domain: 'example.com',
+    url: 'https://example.com/foo.html',
+    isStickerPack: false,
+  };
+
+  const fakeAttachment = (
+    overrides: Partial<AttachmentType> = {}
+  ): AttachmentType => ({
+    contentType: IMAGE_JPEG,
+    fileName: 'foo.jpg',
+    url: '/tmp/foo.jpg',
+    width: 800,
+    height: 600,
+    ...overrides,
+  });
+
+  it('returns false if there is no image', () => {
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+      })
+    );
+  });
+
+  it('returns false is the preview is a sticker pack', () => {
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        isStickerPack: true,
+        image: fakeAttachment(),
+      })
+    );
+  });
+
+  it("returns false if either of the image's dimensions are missing", () => {
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: undefined }),
+      })
+    );
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ height: undefined }),
+      })
+    );
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: undefined, height: undefined }),
+      })
+    );
+  });
+
+  it("returns false if either of the image's dimensions are <200px", () => {
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: 199 }),
+      })
+    );
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ height: 199 }),
+      })
+    );
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: 150, height: 199 }),
+      })
+    );
+  });
+
+  it('returns false if the image is square', () => {
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: 200, height: 200 }),
+      })
+    );
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: 500, height: 500 }),
+      })
+    );
+  });
+
+  it('returns false if the image is roughly square', () => {
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: 200, height: 201 }),
+      })
+    );
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: 497, height: 501 }),
+      })
+    );
+  });
+
+  it("returns false for large attachments that aren't images", () => {
+    assert.isFalse(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({
+          contentType: VIDEO_MP4,
+          fileName: 'foo.mp4',
+          url: '/tmp/foo.mp4',
+        }),
+      })
+    );
+  });
+
+  it('returns true for larger images', () => {
+    assert.isTrue(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment({ width: 200, height: 500 }),
+      })
+    );
+    assert.isTrue(
+      shouldUseFullSizeLinkPreviewImage({
+        ...baseLinkPreview,
+        image: fakeAttachment(),
+      })
+    );
+  });
+});

ts/types/Attachment.ts

@@ -133,9 +133,9 @@ export function isImage(
 }
 
 export function isImageAttachment(
-  attachment: AttachmentType
+  attachment?: AttachmentType
-): boolean | undefined {
+): attachment is AttachmentType {
-  return (
+  return Boolean(
     attachment &&
       attachment.contentType &&
       isImageTypeSupported(attachment.contentType)

ts/types/message/LinkPreviews.ts

@@ -0,0 +1,14 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { AttachmentType } from '../Attachment';
+
+export interface LinkPreviewType {
+  title: string;
+  description?: string;
+  domain: string;
+  url: string;
+  isStickerPack: boolean;
+  image?: AttachmentType;
+  date?: number;
+}

ts/util/lint/exceptions.json

@@ -14782,7 +14782,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.js",
     "line": "        this.audioRef = react_1.default.createRef();",
-    "lineNumber": 62,
+    "lineNumber": 61,
     "reasonCategory": "usageTrusted",
     "updated": "2020-08-28T16:12:19.904Z"
   },
@@ -14790,7 +14790,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.js",
     "line": "        this.focusRef = react_1.default.createRef();",
-    "lineNumber": 63,
+    "lineNumber": 62,
     "reasonCategory": "usageTrusted",
     "updated": "2020-09-11T17:24:56.124Z",
     "reasonDetail": "Used for managing focus only"
@@ -14799,7 +14799,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.js",
     "line": "        this.reactionsContainerRef = react_1.default.createRef();",
-    "lineNumber": 64,
+    "lineNumber": 63,
     "reasonCategory": "usageTrusted",
     "updated": "2020-08-28T16:12:19.904Z",
     "reasonDetail": "Used for detecting clicks outside reaction viewer"
@@ -14808,7 +14808,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.tsx",
     "line": "  public audioRef: React.RefObject<HTMLAudioElement> = React.createRef();",
-    "lineNumber": 226,
+    "lineNumber": 216,
     "reasonCategory": "usageTrusted",
     "updated": "2020-09-08T20:19:01.913Z"
   },
@@ -14816,7 +14816,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.tsx",
     "line": "  public focusRef: React.RefObject<HTMLDivElement> = React.createRef();",
-    "lineNumber": 228,
+    "lineNumber": 218,
     "reasonCategory": "usageTrusted",
     "updated": "2020-09-08T20:19:01.913Z"
   },
@@ -14824,7 +14824,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.tsx",
     "line": "  > = React.createRef();",
-    "lineNumber": 232,
+    "lineNumber": 222,
     "reasonCategory": "usageTrusted",
     "updated": "2020-08-28T19:36:40.817Z"
   },