From 6b656d4381e5b439d171565647b7ca25b4a4a2fe Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 24 Feb 2026 10:50:27 +0000 Subject: [PATCH] Refactors --- .../UrlPreviewView/LinkPreview.stories.tsx | 2 +- .../UrlPreviewGroupView.stories.tsx | 41 ++-- .../UrlPreviewGroupView.test.tsx | 4 +- .../message-body/UrlPreviewViewModel.ts | 213 ++++++++++-------- .../message-body/UrlPreviewViewModel-test.ts | 93 +++++++- 5 files changed, 238 insertions(+), 115 deletions(-) diff --git a/packages/shared-components/src/event-tiles/UrlPreviewView/LinkPreview.stories.tsx b/packages/shared-components/src/event-tiles/UrlPreviewView/LinkPreview.stories.tsx index 7fed04d08b..cce67c95f7 100644 --- a/packages/shared-components/src/event-tiles/UrlPreviewView/LinkPreview.stories.tsx +++ b/packages/shared-components/src/event-tiles/UrlPreviewView/LinkPreview.stories.tsx @@ -20,7 +20,7 @@ export default { onHideClick: fn(), onImageClick: fn(), }, -} as Meta; +} satisfies Meta; const Template: StoryFn = (args) => ; diff --git a/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.stories.tsx b/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.stories.tsx index 81682f11c7..e10e2b840e 100644 --- a/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.stories.tsx +++ b/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.stories.tsx @@ -42,7 +42,7 @@ export default { onImageClick: fn(), onTogglePreviewLimit: fn(), }, -} as Meta; +} satisfies Meta; const Template: StoryFn = (args) => ; @@ -61,8 +61,26 @@ Default.args = { ], }; -export const MultiplePreviews = Template.bind({}); -MultiplePreviews.args = { +export const MultiplePreviewsHidden = Template.bind({}); +MultiplePreviewsHidden.args = { + previews: [ + { + title: "A simple title", + description: "A simple description", + link: "https://matrix.org", + image: { + imageThumb: imageFile, + imageFull: imageFile, + }, + }, + ], + overPreviewLimit: true, + previewsLimited: true, + totalPreviewCount: 10, +}; + +export const MultiplePreviewsVisible = Template.bind({}); +MultiplePreviewsVisible.args = { previews: [ { title: "One", @@ -92,22 +110,7 @@ MultiplePreviews.args = { }, }, ], -}; - -export const MultiplePreviewsHidden = Template.bind({}); -MultiplePreviewsHidden.args = { - previews: [ - { - title: "A simple title", - description: "A simple description", - link: "https://matrix.org", - image: { - imageThumb: imageFile, - imageFull: imageFile, - }, - }, - ], overPreviewLimit: true, - previewsLimited: true, + previewsLimited: false, totalPreviewCount: 10, }; diff --git a/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.test.tsx b/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.test.tsx index ea01b6a770..b009876c32 100644 --- a/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.test.tsx +++ b/packages/shared-components/src/event-tiles/UrlPreviewView/UrlPreviewGroupView.test.tsx @@ -12,7 +12,7 @@ import React from "react"; import * as stories from "./UrlPreviewGroupView.stories.tsx"; -const { Default, MultiplePreviews, MultiplePreviewsHidden } = composeStories(stories); +const { Default, MultiplePreviewsHidden, MultiplePreviewsVisible } = composeStories(stories); describe("UrlPreviewGroupView", () => { it("renders a single preview", () => { @@ -20,7 +20,7 @@ describe("UrlPreviewGroupView", () => { expect(container).toMatchSnapshot(); }); it("renders multiple previews", () => { - const { container } = render(); + const { container } = render(); expect(container).toMatchSnapshot(); }); it("renders multiple previews which are hidden", () => { diff --git a/src/viewmodels/message-body/UrlPreviewViewModel.ts b/src/viewmodels/message-body/UrlPreviewViewModel.ts index 08a933ee41..b0bba52b83 100644 --- a/src/viewmodels/message-body/UrlPreviewViewModel.ts +++ b/src/viewmodels/message-body/UrlPreviewViewModel.ts @@ -16,7 +16,6 @@ import { type IPreviewUrlResponse, type MatrixClient, MatrixError, type MatrixEv import { isPermalinkHost } from "../../utils/permalinks/Permalinks"; import { mediaFromMxc } from "../../customisations/Media"; -import { linkifyAndSanitizeHtml } from "../../Linkify"; import PlatformPeg from "../../PlatformPeg"; import { thumbHeight } from "../../ImageUtils"; import SettingsStore from "../../settings/SettingsStore"; @@ -35,31 +34,6 @@ export const MAX_PREVIEWS_WHEN_LIMITED = 2; export const PREVIEW_WIDTH = 100; export const PREVIEW_HEIGHT = 100; -function getNumberFromOpenGraph(value: number | string | undefined): number | undefined { - if (typeof value === "number") { - return value; - } else if (typeof value === "string" && value) { - const i = parseInt(value, 10); - if (!isNaN(i)) { - return i; - } - } - return undefined; -} - -function getTitleFromOpenGraph(response: IPreviewUrlResponse, link: string): string { - if (typeof response["og:title"] === "string" && response["og:title"]) { - return response["og:title"].trim(); - } - if (typeof response["og:site_name"] === "string" && response["og:site_name"]) { - return response["og:site_name"].trim(); - } - if (typeof response["og:description"] === "string" && response["og:description"]) { - return response["og:description"].trim(); - } - return link; -} - export enum PreviewVisibility { /** * Preview is entirely hidden from view and can not be changed. @@ -87,43 +61,102 @@ export class UrlPreviewViewModel implements UrlPreviewGroupViewActions { /** - * Determine if an anchor element can be rendered into a preview. - * @param node The anchor element DOM node. + * Parse a numeric value from OpenGraph. The OpenGraph spec defines all values as strings + * although Synapse may return these values as numbers. To be compatible, test strings + * and numbers. + * @param value The numeric value + * @returns A number if the value parsed correctly, or undefined otherwise. */ - private static isLinkPreviewable(node: HTMLAnchorElement): boolean { + private static getNumberFromOpenGraph(value: number | string | undefined): number | undefined { + if (typeof value === "number") { + return value; + } else if (typeof value === "string" && value) { + const i = parseInt(value, 10); + if (!isNaN(i)) { + return i; + } + } + return undefined; + } + + /** + * Calculate the best possible title from an opengraph response. + * @param response The opengraph response + * @param link The link being used to preview. + * @returns The title value. + */ + private static getBaseMetadataFromResponse( + response: IPreviewUrlResponse, + link: string, + ): Pick { + let title = + typeof response["og:title"] === "string" && response["og:title"].trim() + ? response["og:title"].trim() + : undefined; + let description = + typeof response["og:description"] === "string" && response["og:description"].trim() + ? response["og:description"].trim() + : undefined; + let siteName = + typeof response["og:site_name"] === "string" && response["og:site_name"].trim() + ? response["og:site_name"].trim() + : undefined; + + if (!title && description) { + title = description; + description = undefined; + } else if (!title && siteName) { + title = siteName; + siteName = undefined; + } else if (!title) { + title = link; + } + + return { + title, + description, + siteName, + }; + } + + /** + * Determine if an anchor element can be rendered into a preview. + * If it can, return the value of `href` + * @param node The anchor element DOM node. + * @returns The value of the `href` of the node, or null if this node cannot be previewed. + */ + private static getAnchorLink(node: HTMLAnchorElement): string | null { // don't try to preview relative links const href = node.getAttribute("href"); if (!href || !URL.canParse(href)) { - return false; + return null; } const url = new URL(href); if (!["http:", "https:"].includes(url.protocol)) { - return false; + return null; } // never preview permalinks (if anything we should give a smart // preview of the room/user they point to: nobody needs to be reminded // what the matrix.to site looks like). if (isPermalinkHost(url.host)) { - return false; + return null; } // as a random heuristic to avoid highlighting things like "foo.pl" // we require the linked text to either include a / (either from http:// // or from a full foo.bar/baz style schemeless URL) - or be a markdown-style // link, in which case we check the target text differs from the link value. - // TODO: make this configurable? if (node.textContent?.includes("/")) { - return true; + return href; } if (node.textContent?.toLowerCase().trim().startsWith(url.host.toLowerCase())) { // it's a "foo.pl" style link - return false; - } else { - // it's a [foo bar](http://foo.com) style link - return true; + return null; } + // it's a [foo bar](http://foo.com) style link + return href; } /** @@ -131,19 +164,19 @@ export class UrlPreviewViewModel * @param nodes An array of DOM elements that may be or contain anchor elements. * @returns A unique array of links that can be previewed, in order of discovery. */ - private static findLinks(nodes: ArrayLike): string[] { + private static findLinks(nodes: Iterable): string[] { let links = new Set(); - for (let i = 0; i < nodes.length; i++) { - const node = nodes[i]; - if (node.tagName === "A" && node.getAttribute("href")) { - if (this.isLinkPreviewable(node as HTMLAnchorElement)) { - links.add(node.getAttribute("href")!); + for (const node of nodes) { + if (node.tagName === "A") { + const href = this.getAnchorLink(node as HTMLAnchorElement); + if (href) { + links.add(href); } } else if (node.tagName === "PRE" || node.tagName === "CODE" || node.tagName === "BLOCKQUOTE") { continue; } else if (node.children && node.children.length) { - links = new Set([...this.findLinks(node.children), ...links]); + links = new Set([...links, ...this.findLinks(node.children)]); } } return [...links]; @@ -161,52 +194,10 @@ export class UrlPreviewViewModel if (cached) { return cached; } + let preview: IPreviewUrlResponse; + try { - const preview = await this.client.getUrlPreview(link, this.eventSendTime); - const title = getTitleFromOpenGraph(preview, link); - const hasImage = preview["og:image"] && typeof preview?.["og:image"] === "string"; - // Ensure we have something relevant to render. - // The title must not just be the link, or we must have an image. - if (title === link && !hasImage) { - return null; - } - - const media = - typeof preview["og:image"] === "string" && this.visibility > PreviewVisibility.MediaHidden - ? mediaFromMxc(preview["og:image"], this.client) - : undefined; - const needsTooltip = link !== title && PlatformPeg.get()?.needsUrlTooltips(); - - // TODO: Magic numbers - const imageMaxHeight = 100; - const declaredHeight = getNumberFromOpenGraph(preview["og:image:height"]); - const width = Math.min(getNumberFromOpenGraph(preview["og:image:width"]) || 101, 100); - // TODO: This is wrong. - const height = thumbHeight(width, declaredHeight, imageMaxHeight, imageMaxHeight) ?? imageMaxHeight; - - const result = { - link, - title, - siteName: typeof preview["og:site_name"] === "string" ? preview["og:site_name"] : undefined, - showTooltipOnLink: needsTooltip, - // Don't show a description if it's the same as the title. - description: - typeof preview["og:description"] === "string" && title !== preview["og:description"] - ? linkifyAndSanitizeHtml(preview["og:description"]) - : undefined, - image: media - ? { - // TODO: Check nulls - imageThumb: media.getThumbnailOfSourceHttp(PREVIEW_WIDTH, PREVIEW_HEIGHT, "scale")!, - imageFull: media.srcHttp!, - width, - height, - fileSize: getNumberFromOpenGraph(preview["matrix:image:size"]), - } - : undefined, - } satisfies UrlPreviewViewSnapshotPreview; - this.previewCache.set(link, result); - return result; + preview = await this.client.getUrlPreview(link, this.eventSendTime); } catch (error) { if (error instanceof MatrixError && error.httpStatus === 404) { // Quieten 404 Not found errors, not all URLs can have a preview generated @@ -214,8 +205,46 @@ export class UrlPreviewViewModel } else { logger.error("Failed to get URL preview: ", error); } + return null; } - return null; + + const { title, description, siteName } = UrlPreviewViewModel.getBaseMetadataFromResponse(preview, link); + const hasImage = preview["og:image"] && typeof preview?.["og:image"] === "string"; + // Ensure we have something relevant to render. + // The title must not just be the link, or we must have an image. + if (title === link && !hasImage) { + return null; + } + let image: UrlPreviewViewSnapshotPreview["image"]; + if (typeof preview["og:image"] === "string" && this.visibility > PreviewVisibility.MediaHidden) { + const media = mediaFromMxc(preview["og:image"], this.client); + const declaredHeight = UrlPreviewViewModel.getNumberFromOpenGraph(preview["og:image:height"]); + const declaredWidth = UrlPreviewViewModel.getNumberFromOpenGraph(preview["og:image:width"]); + const width = Math.min(declaredWidth ?? PREVIEW_WIDTH, PREVIEW_WIDTH); + const height = thumbHeight(width, declaredHeight, PREVIEW_WIDTH, PREVIEW_WIDTH) ?? PREVIEW_WIDTH; + const thumb = media.getThumbnailOfSourceHttp(PREVIEW_WIDTH, PREVIEW_HEIGHT, "scale"); + // No thumb, no preview. + if (thumb) { + image = { + imageThumb: thumb, + imageFull: media.srcHttp ?? thumb, + width, + height, + fileSize: UrlPreviewViewModel.getNumberFromOpenGraph(preview["matrix:image:size"]), + }; + } + } + + const result = { + link, + title, + description, + siteName, + showTooltipOnLink: link !== title && PlatformPeg.get()?.needsUrlTooltips(), + image, + } satisfies UrlPreviewViewSnapshotPreview; + this.previewCache.set(link, result); + return result; } private readonly client: MatrixClient; @@ -250,7 +279,7 @@ export class UrlPreviewViewModel /** * A cache containing all previously calculated previews. */ - private previewCache = new Map(); + private readonly previewCache = new Map(); /** * Callback for when the image element is clicked on. diff --git a/test/viewmodels/message-body/UrlPreviewViewModel-test.ts b/test/viewmodels/message-body/UrlPreviewViewModel-test.ts index 184d633971..be81ef5442 100644 --- a/test/viewmodels/message-body/UrlPreviewViewModel-test.ts +++ b/test/viewmodels/message-body/UrlPreviewViewModel-test.ts @@ -8,7 +8,7 @@ import { expect } from "@jest/globals"; import type { MockedObject } from "jest-mock"; -import type { MatrixClient } from "matrix-js-sdk/src/matrix"; +import type { MatrixClient, IPreviewUrlResponse } from "matrix-js-sdk/src/matrix"; import { UrlPreviewViewModel } from "../../../src/viewmodels/message-body/UrlPreviewViewModel"; import type { UrlPreviewViewSnapshotPreview } from "@element-hq/web-shared-components"; import { getMockClientWithEventEmitter, mkEvent } from "../../test-utils"; @@ -81,6 +81,39 @@ describe("UrlPreviewViewModel", () => { totalPreviewCount: 1, }); }); + it("should preview nested URLs but ignore some element types", async () => { + const { vm, client } = getViewModel(); + vm.onTogglePreviewLimit(); + client.getUrlPreview.mockResolvedValue(BASIC_PREVIEW_OGDATA); + const msg = document.createElement("div"); + msg.innerHTML = ` + + 
Test4
+ Test5 +
Test6
`; + await vm.updateEventElement(msg); + const { previews } = vm.getSnapshot(); + expect(previews).toHaveLength(3); + expect(previews).toMatchObject([ + { + link: "https://example.org/1", + }, + { + link: "https://example.org/2", + }, + { + link: "https://example.org/3", + }, + ]); + }); it("should hide preview when invisible", async () => { const { vm, client } = getViewModel({ visible: false, mediaVisible: true }); const msg = document.createElement("div"); @@ -286,4 +319,62 @@ describe("UrlPreviewViewModel", () => { await vm.updateEventElement(msg); expect(vm.getSnapshot().previews).toHaveLength(item.hasPreview ? 1 : 0); }); + + // og:url, og:type are ignored. + const baseOg = { + "og:url": "https://example.org", + "og:type": "document", + }; + + it.each<[IPreviewUrlResponse, Omit]>([ + [{ ...baseOg, "og:title": "Basic title" }, { title: "Basic title" }], + [ + { ...baseOg, "og:site_name": "Site name", "og:title": "" }, + { title: "Site name", siteName: undefined }, + ], + [ + { ...baseOg, "og:description": "A description", "og:title": "" }, + { title: "A description", description: undefined }, + ], + [ + { ...baseOg, "og:title": "Cool blog", "og:site_name": "Cool site" }, + { title: "Cool blog", siteName: "Cool site" }, + ], + [ + { + ...baseOg, + "og:title": "Media test", + // API *may* return a string, so check we parse correctly. + "og:image:height": "500" as unknown as number, + "og:image:width": 500, + "matrix:image:size": 1024, + "og:image": IMAGE_MXC, + }, + { + title: "Media test", + image: { + imageThumb: "https://example.org/image/thumb", + imageFull: "https://example.org/image/src", + fileSize: 1024, + width: 100, + height: 100, + }, + }, + ], + ])("handles different kinds of opengraph responses %s", async (og, preview) => { + const { vm, client } = getViewModel(); + // eslint-disable-next-line no-restricted-properties + client.mxcUrlToHttp.mockImplementation((url, width) => { + expect(url).toEqual(IMAGE_MXC); + if (width) { + return "https://example.org/image/thumb"; + } + return "https://example.org/image/src"; + }); + client.getUrlPreview.mockResolvedValueOnce(og); + const msg = document.createElement("div"); + msg.innerHTML = `test`; + await vm.updateEventElement(msg); + expect(vm.getSnapshot().previews[0]).toMatchObject(preview); + }); });