mirror of
https://github.com/block/buzz.git
synced 2026-10-31 22:41:12 +00:00
fix(desktop): preserve rich link preview intent
Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
This commit is contained in:
@@ -6,8 +6,11 @@ const MAX_TITLE: usize = 300;
|
||||
const MAX_SITE: usize = 100;
|
||||
const MAX_DESCRIPTION: usize = 1000;
|
||||
|
||||
fn valid_text(value: &str, max: usize) -> bool {
|
||||
value.len() <= max && !value.chars().any(char::is_control)
|
||||
fn valid_text(value: &str, max: usize, allow_newlines: bool) -> bool {
|
||||
value.len() <= max
|
||||
&& !value
|
||||
.chars()
|
||||
.any(|character| character.is_control() && !(allow_newlines && character == '\n'))
|
||||
}
|
||||
|
||||
fn valid_sha256(value: &str) -> bool {
|
||||
@@ -78,9 +81,9 @@ pub fn append(
|
||||
&& url.fragment().is_none()
|
||||
})
|
||||
&& seen.insert(preview_tag[3].clone())
|
||||
&& valid_text(&preview_tag[4], MAX_TITLE)
|
||||
&& valid_text(&preview_tag[5], MAX_SITE)
|
||||
&& valid_text(&preview_tag[6], MAX_DESCRIPTION)
|
||||
&& valid_text(&preview_tag[4], MAX_TITLE, false)
|
||||
&& valid_text(&preview_tag[5], MAX_SITE, false)
|
||||
&& valid_text(&preview_tag[6], MAX_DESCRIPTION, true)
|
||||
&& valid_media_pair(&preview_tag[7], &preview_tag[8], &base)
|
||||
&& valid_media_pair(&preview_tag[9], &preview_tag[10], &base);
|
||||
if !valid {
|
||||
@@ -147,6 +150,20 @@ mod tests {
|
||||
.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_accepts_description_newlines() {
|
||||
let mut preview_tag = tag("", "");
|
||||
preview_tag[6] = "First paragraph\n\nSecond paragraph".into();
|
||||
assert!(append(&[preview_tag], BASE, &mut Vec::new()).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_rejects_other_control_characters() {
|
||||
let mut preview_tag = tag("", "");
|
||||
preview_tag[6] = "Unsafe\tdescription".into();
|
||||
assert!(append(&[preview_tag], BASE, &mut Vec::new()).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_rejects_untrusted_or_malformed_snapshot_media() {
|
||||
for url in [
|
||||
|
||||
24
desktop/src/features/messages/lib/linkPreviewContent.ts
Normal file
24
desktop/src/features/messages/lib/linkPreviewContent.ts
Normal file
@@ -0,0 +1,24 @@
|
||||
import type { Node as ProseMirrorNode } from "@tiptap/pm/model";
|
||||
|
||||
import { buildPlainTextProjection } from "./plainTextProjection";
|
||||
|
||||
export function buildPreviewUpdate(
|
||||
doc: ProseMirrorNode,
|
||||
selectionAnchor: number,
|
||||
) {
|
||||
const projection = buildPlainTextProjection(doc);
|
||||
const plainText = projection.text;
|
||||
const hrefs = new Set<string>();
|
||||
doc.descendants((node) => {
|
||||
if (!node.isText || node.marks.some((mark) => mark.type.name === "spoiler"))
|
||||
return;
|
||||
const href = node.marks.find((mark) => mark.type.name === "link")?.attrs
|
||||
.href;
|
||||
if (typeof href === "string") hrefs.add(href);
|
||||
});
|
||||
return {
|
||||
cursor: projection.mapPMToTextOffset(selectionAnchor),
|
||||
linkPreviewContent: [plainText, ...hrefs].join("\n"),
|
||||
text: plainText,
|
||||
};
|
||||
}
|
||||
@@ -29,6 +29,7 @@ import {
|
||||
import { CUSTOM_EMOJI_NODE_NAME } from "./customEmojiNode";
|
||||
import { useComposerCustomEmoji } from "./useComposerCustomEmoji";
|
||||
import { buildPlainTextProjection } from "./plainTextProjection";
|
||||
import { buildPreviewUpdate } from "./linkPreviewContent";
|
||||
import { createLinkInteractionExtension } from "./linkInteractionExtension";
|
||||
import {
|
||||
CodeBlockAfterHardBreak,
|
||||
@@ -76,7 +77,7 @@ export type AutocompleteEdit = {
|
||||
|
||||
export type RichTextEditorOptions = {
|
||||
placeholder?: string;
|
||||
onUpdate?: (info: { text: string; cursor: number }) => void;
|
||||
onUpdate?: (info: ReturnType<typeof buildPreviewUpdate>) => void;
|
||||
editable?: boolean;
|
||||
mentionNames?: string[];
|
||||
agentMentionNames?: string[];
|
||||
@@ -654,11 +655,9 @@ export function useRichTextEditor({
|
||||
// still available through `getMarkdown()` for send/draft boundaries;
|
||||
// per-keystroke consumers only need textarea-shaped plain text for
|
||||
// autocomplete and empty/non-empty state.
|
||||
const projection = buildPlainTextProjection(ed.state.doc);
|
||||
onUpdateRef.current?.({
|
||||
cursor: projection.mapPMToTextOffset(ed.state.selection.anchor),
|
||||
text: projection.text,
|
||||
});
|
||||
onUpdateRef.current?.(
|
||||
buildPreviewUpdate(ed.state.doc, ed.state.selection.anchor),
|
||||
);
|
||||
},
|
||||
},
|
||||
[],
|
||||
|
||||
@@ -262,9 +262,9 @@ function MessageComposerImpl({
|
||||
onEditLink: (info) => onEditLinkRef.current?.(info),
|
||||
onLinkSelectionChange: (info) => onLinkSelectionChangeRef.current?.(info),
|
||||
onLinkShortcut: () => onLinkShortcutRef.current?.() ?? false,
|
||||
onUpdate: ({ cursor, text }) => {
|
||||
onUpdate: ({ cursor, linkPreviewContent, text }) => {
|
||||
setComposerContentFromText(text);
|
||||
setPreviewContent(text);
|
||||
setPreviewContent(linkPreviewContent);
|
||||
mentions.updateMentionQuery(text, cursor);
|
||||
channelLinks.updateChannelQuery(text, cursor);
|
||||
emojiAutocomplete.updateEmojiQuery(text, cursor);
|
||||
|
||||
@@ -13,6 +13,16 @@ import {
|
||||
} from "@/shared/lib/mediaUrl";
|
||||
import type { ResolvedLinkPreview } from "@/shared/lib/useResolvedLinkPreviews";
|
||||
import { useResolvedLinkPreviews } from "@/shared/lib/useResolvedLinkPreviews";
|
||||
import {
|
||||
AlertDialog,
|
||||
AlertDialogAction,
|
||||
AlertDialogCancel,
|
||||
AlertDialogContent,
|
||||
AlertDialogDescription,
|
||||
AlertDialogFooter,
|
||||
AlertDialogHeader,
|
||||
AlertDialogTitle,
|
||||
} from "@/shared/ui/alert-dialog";
|
||||
import {
|
||||
Attachment,
|
||||
AttachmentContent,
|
||||
@@ -133,6 +143,7 @@ async function uploadDataUrl(
|
||||
|
||||
export function useComposerLinkPreviews(content: string) {
|
||||
const [suppressed, setSuppressed] = React.useState(false);
|
||||
const [dismissDialogOpen, setDismissDialogOpen] = React.useState(false);
|
||||
const candidates = React.useMemo(
|
||||
() =>
|
||||
extractSupportedLinkPreviews(content).filter((preview) =>
|
||||
@@ -213,33 +224,62 @@ export function useComposerLinkPreviews(content: string) {
|
||||
: candidates.flatMap((candidate) =>
|
||||
readyTags[candidate.href] ? [readyTags[candidate.href]] : [],
|
||||
);
|
||||
const hideAll = React.useCallback(() => setSuppressed(true), []);
|
||||
const previewNoun = previews.length === 1 ? "preview" : "previews";
|
||||
const hideAll = React.useCallback(() => {
|
||||
setSuppressed(true);
|
||||
setDismissDialogOpen(false);
|
||||
}, []);
|
||||
const previewList = previews.length ? (
|
||||
<div
|
||||
className="mb-2"
|
||||
data-composer-link-previews=""
|
||||
data-ready-snapshot-count={readyTagsRef.current.length}
|
||||
>
|
||||
<div className="flex max-w-full items-start gap-1">
|
||||
<AttachmentGroup className="max-w-full flex-row flex-wrap items-start overflow-visible pb-0">
|
||||
{previews.map((preview) => (
|
||||
<ComposerLinkPreviewCard key={preview.href} preview={preview} />
|
||||
))}
|
||||
</AttachmentGroup>
|
||||
<Button
|
||||
aria-label="Hide all link previews"
|
||||
className="mt-1 size-5 shrink-0 rounded-full text-muted-foreground hover:text-foreground [&_svg]:size-3"
|
||||
data-testid="composer-hide-link-previews"
|
||||
onClick={hideAll}
|
||||
size="icon-xs"
|
||||
title="Hide previews"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<X aria-hidden="true" />
|
||||
</Button>
|
||||
<>
|
||||
<div
|
||||
className="mb-2"
|
||||
data-composer-link-previews=""
|
||||
data-ready-snapshot-count={readyTagsRef.current.length}
|
||||
>
|
||||
<div className="flex max-w-full items-start gap-1">
|
||||
<AttachmentGroup className="max-w-full flex-row flex-wrap items-start overflow-visible pb-0">
|
||||
{previews.map((preview) => (
|
||||
<ComposerLinkPreviewCard key={preview.href} preview={preview} />
|
||||
))}
|
||||
</AttachmentGroup>
|
||||
<Button
|
||||
aria-label="Hide all link previews"
|
||||
className="mt-1 size-5 shrink-0 rounded-full text-muted-foreground hover:text-foreground [&_svg]:size-3"
|
||||
data-testid="composer-hide-link-previews"
|
||||
onClick={() => setDismissDialogOpen(true)}
|
||||
size="icon-xs"
|
||||
title="Hide previews"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<X aria-hidden="true" />
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<AlertDialog onOpenChange={setDismissDialogOpen} open={dismissDialogOpen}>
|
||||
<AlertDialogContent>
|
||||
<AlertDialogHeader>
|
||||
<AlertDialogTitle>Hide link {previewNoun}?</AlertDialogTitle>
|
||||
<AlertDialogDescription>
|
||||
{previews.length === 1 ? "This preview" : "These previews"} won’t
|
||||
be included when you send this message.
|
||||
</AlertDialogDescription>
|
||||
</AlertDialogHeader>
|
||||
<AlertDialogFooter>
|
||||
<AlertDialogCancel asChild>
|
||||
<Button type="button" variant="outline">
|
||||
Cancel
|
||||
</Button>
|
||||
</AlertDialogCancel>
|
||||
<AlertDialogAction asChild>
|
||||
<Button onClick={hideAll} type="button" variant="destructive">
|
||||
Hide {previewNoun}
|
||||
</Button>
|
||||
</AlertDialogAction>
|
||||
</AlertDialogFooter>
|
||||
</AlertDialogContent>
|
||||
</AlertDialog>
|
||||
</>
|
||||
) : null;
|
||||
const getReadyTags = React.useCallback(() => {
|
||||
if (suppressedRef.current) return [["link-preview", "none"]];
|
||||
|
||||
@@ -63,7 +63,7 @@ test("snapshot tags sanitize control characters and enforce UTF-8 byte limits",
|
||||
canonicalUrl: URL,
|
||||
title: `Title\n${"😀".repeat(100)}`,
|
||||
siteName: `Linear\u0000${"é".repeat(100)}`,
|
||||
description: `First\nSecond${"😀".repeat(300)}`,
|
||||
description: `First\nSecond\t${"😀".repeat(300)}`,
|
||||
imageUrl: "",
|
||||
imageSha256: "",
|
||||
faviconUrl: "",
|
||||
@@ -71,7 +71,9 @@ test("snapshot tags sanitize control characters and enforce UTF-8 byte limits",
|
||||
});
|
||||
|
||||
assert.ok(tag);
|
||||
for (const value of tag.slice(4, 7)) {
|
||||
assert.equal(tag[6].startsWith("First\nSecond"), true);
|
||||
assert.equal(tag[6].includes("\t"), false);
|
||||
for (const value of tag.slice(4, 6)) {
|
||||
assert.ok(
|
||||
Array.from(value).every((char) => {
|
||||
const code = char.charCodeAt(0);
|
||||
|
||||
@@ -32,16 +32,21 @@ export type LinkPreviewSnapshot = {
|
||||
faviconSha256: string;
|
||||
};
|
||||
|
||||
function isControlCharacter(char: string): boolean {
|
||||
function isControlCharacter(char: string, allowNewlines = false): boolean {
|
||||
if (allowNewlines && char === "\n") return false;
|
||||
const code = char.charCodeAt(0);
|
||||
return code <= 0x1f || code === 0x7f;
|
||||
}
|
||||
|
||||
function sanitizeSnapshotText(value: string, maxBytes: number): string {
|
||||
function sanitizeSnapshotText(
|
||||
value: string,
|
||||
maxBytes: number,
|
||||
allowNewlines = false,
|
||||
): string {
|
||||
let result = "";
|
||||
let byteLength = 0;
|
||||
for (const rawChar of value) {
|
||||
const char = isControlCharacter(rawChar) ? " " : rawChar;
|
||||
const char = isControlCharacter(rawChar, allowNewlines) ? " " : rawChar;
|
||||
const charBytes = new TextEncoder().encode(char).length;
|
||||
if (byteLength + charBytes > maxBytes) break;
|
||||
result += char;
|
||||
@@ -50,12 +55,11 @@ function sanitizeSnapshotText(value: string, maxBytes: number): string {
|
||||
return result;
|
||||
}
|
||||
|
||||
function validText(value: string, max: number): boolean {
|
||||
function validText(value: string, max: number, allowNewlines = false): boolean {
|
||||
return (
|
||||
new TextEncoder().encode(value).length <= max &&
|
||||
!Array.from(value).some((char) => {
|
||||
const code = char.charCodeAt(0);
|
||||
return code <= 0x1f || code === 0x7f;
|
||||
return isControlCharacter(char, allowNewlines);
|
||||
})
|
||||
);
|
||||
}
|
||||
@@ -131,7 +135,7 @@ export function parseLinkPreviewSnapshots(
|
||||
if (
|
||||
!validText(title, 300) ||
|
||||
!validText(siteName, 100) ||
|
||||
!validText(description, 1000)
|
||||
!validText(description, 1000, true)
|
||||
)
|
||||
continue;
|
||||
if (
|
||||
@@ -166,7 +170,7 @@ export function buildLinkPreviewSnapshotTag(
|
||||
snapshot.canonicalUrl,
|
||||
sanitizeSnapshotText(snapshot.title, 300),
|
||||
sanitizeSnapshotText(snapshot.siteName, 100),
|
||||
sanitizeSnapshotText(snapshot.description, 1000),
|
||||
sanitizeSnapshotText(snapshot.description, 1000, true),
|
||||
snapshot.imageUrl,
|
||||
snapshot.imageSha256,
|
||||
snapshot.faviconUrl,
|
||||
|
||||
@@ -206,32 +206,46 @@ test.beforeEach(async ({ page }, testInfo) => {
|
||||
},
|
||||
linkPreviewMetadataDelayMs: 2_000,
|
||||
}
|
||||
: testInfo.title.includes("link preview") ||
|
||||
testInfo.title.includes("supported Compact")
|
||||
: testInfo.title.includes(
|
||||
"rich link preview preserves description newlines",
|
||||
)
|
||||
? {
|
||||
linkPreviewMetadata: {
|
||||
title: "Buzz pull request",
|
||||
siteName: "GitHub",
|
||||
description: "A sender-authored preview snapshot.",
|
||||
description:
|
||||
"First paragraph line one.\nFirst paragraph line two.\n\nSecond paragraph.",
|
||||
imageDataUrl: null,
|
||||
imageDomain: null,
|
||||
},
|
||||
linkPreviewMetadataDelayMs: testInfo.title.includes(
|
||||
"loading card before cold resolver work",
|
||||
)
|
||||
? 10_000
|
||||
: testInfo.title.includes("style defaults") ||
|
||||
testInfo.title.includes("send does not wait") ||
|
||||
testInfo.title.includes("attachment-sized")
|
||||
? 1_500
|
||||
: undefined,
|
||||
linkPreviewMetadataStartBlockMs: testInfo.title.includes(
|
||||
"loading card before cold resolver work",
|
||||
)
|
||||
? 150
|
||||
: undefined,
|
||||
}
|
||||
: undefined;
|
||||
: testInfo.title.includes("link preview") ||
|
||||
testInfo.title.includes("supported Compact")
|
||||
? {
|
||||
linkPreviewMetadata: {
|
||||
title: "Buzz pull request",
|
||||
siteName: "GitHub",
|
||||
description: "A sender-authored preview snapshot.",
|
||||
imageDataUrl: null,
|
||||
imageDomain: null,
|
||||
},
|
||||
linkPreviewMetadataDelayMs: testInfo.title.includes(
|
||||
"loading card before cold resolver work",
|
||||
)
|
||||
? 10_000
|
||||
: testInfo.title.includes("style defaults") ||
|
||||
testInfo.title.includes("send does not wait") ||
|
||||
testInfo.title.includes("attachment-sized")
|
||||
? 1_500
|
||||
: undefined,
|
||||
linkPreviewMetadataStartBlockMs:
|
||||
testInfo.title.includes(
|
||||
"loading card before cold resolver work",
|
||||
)
|
||||
? 150
|
||||
: undefined,
|
||||
}
|
||||
: undefined;
|
||||
const mock = testInfo.title.includes("unresolvable preview")
|
||||
? { linkPreviewMetadata: null, linkPreviewMetadataDelayMs: 150 }
|
||||
: baseMock;
|
||||
@@ -535,6 +549,69 @@ for (const [pasteShape, wrapUrl] of [
|
||||
});
|
||||
}
|
||||
|
||||
test("display-text link preview produces and sends its preview", async ({
|
||||
page,
|
||||
}) => {
|
||||
const previewUrl = "https://github.com/block/buzz/pull/3246?display=text";
|
||||
await page.goto("/");
|
||||
await page.getByTestId("channel-general").click();
|
||||
|
||||
const input = page.getByTestId("message-input");
|
||||
await input.click();
|
||||
await input.pressSequentially("review the pull request");
|
||||
await page.keyboard.press("ControlOrMeta+a");
|
||||
await page.keyboard.press("ControlOrMeta+k");
|
||||
const dialog = page.getByRole("dialog", { name: "Add link" });
|
||||
await dialog.getByLabel("URL").fill(previewUrl);
|
||||
await dialog.getByRole("button", { name: "Save" }).click();
|
||||
|
||||
await expect(input.locator(`a[href="${previewUrl}"]`)).toHaveText(
|
||||
"review the pull request",
|
||||
);
|
||||
await expect(page.locator("[data-composer-link-previews]")).toBeVisible();
|
||||
await waitForReadyComposerSnapshots(page);
|
||||
await page.getByTestId("send-message").click();
|
||||
|
||||
const row = page.getByTestId("message-row").last();
|
||||
await expect(
|
||||
row.getByRole("link", { name: "review the pull request", exact: true }),
|
||||
).toHaveAttribute("href", previewUrl);
|
||||
await expect(row.locator("[data-link-preview]")).toBeVisible();
|
||||
const linkPreviewTags = await page.evaluate(() => {
|
||||
const call = [...(window.__BUZZ_E2E_COMMAND_PAYLOADS__ ?? [])]
|
||||
.reverse()
|
||||
.find((entry) => entry.command === "send_channel_message");
|
||||
return (
|
||||
call?.payload as { linkPreviewTags?: string[][] | null } | undefined
|
||||
)?.linkPreviewTags;
|
||||
});
|
||||
expect(linkPreviewTags?.map((tag) => tag[3])).toEqual([previewUrl]);
|
||||
});
|
||||
|
||||
test("rich link preview preserves description newlines after sending", async ({
|
||||
page,
|
||||
}) => {
|
||||
const previewUrl = "https://github.com/block/buzz/pull/3246?newlines=1";
|
||||
await page.addInitScript(() =>
|
||||
localStorage.setItem("buzz.appearance.linkPreviewStyle", "rich"),
|
||||
);
|
||||
await page.goto("/");
|
||||
await page.getByTestId("channel-general").click();
|
||||
await page.getByTestId("message-input").fill(previewUrl);
|
||||
await waitForReadyComposerSnapshots(page);
|
||||
await page.getByTestId("send-message").click();
|
||||
|
||||
const description = page
|
||||
.getByTestId("message-row")
|
||||
.last()
|
||||
.locator('[data-link-preview-inline] [data-slot="attachment-description"]');
|
||||
await expect(description.locator("p")).toHaveCount(2);
|
||||
await expect(description).toHaveText(
|
||||
"First paragraph line one.\nFirst paragraph line two.\n\nSecond paragraph.",
|
||||
{ useInnerText: true },
|
||||
);
|
||||
});
|
||||
|
||||
test("completed link previews send when one URL has an unsnapshotable fragment", async ({
|
||||
page,
|
||||
}) => {
|
||||
@@ -643,7 +720,7 @@ test("send does not wait for a pending link preview snapshot", async ({
|
||||
expect(linkPreviewTags ?? []).toEqual([]);
|
||||
});
|
||||
|
||||
test("hiding composer previews suppresses the whole draft and emits the blanket marker", async ({
|
||||
test("hiding composer link previews suppresses the whole draft and emits the blanket marker", async ({
|
||||
page,
|
||||
}) => {
|
||||
const firstUrl = "https://github.com/block/buzz/pull/3246?hide=all";
|
||||
@@ -652,8 +729,18 @@ test("hiding composer previews suppresses the whole draft and emits the blanket
|
||||
await page.getByTestId("channel-general").click();
|
||||
await page.getByTestId("message-input").fill(firstUrl);
|
||||
await expect(page.locator("[data-composer-link-previews]")).toBeVisible();
|
||||
await waitForReadyComposerSnapshots(page);
|
||||
|
||||
await page.getByTestId("composer-hide-link-previews").click();
|
||||
const dismissDialog = page.getByRole("alertdialog", {
|
||||
name: "Hide link preview?",
|
||||
});
|
||||
await expect(dismissDialog).toBeVisible();
|
||||
await dismissDialog.getByRole("button", { name: "Cancel" }).click();
|
||||
await expect(page.locator("[data-composer-link-previews]")).toBeVisible();
|
||||
|
||||
await page.getByTestId("composer-hide-link-previews").click();
|
||||
await dismissDialog.getByRole("button", { name: "Hide preview" }).click();
|
||||
await expect(page.locator("[data-composer-link-previews]")).toHaveCount(0);
|
||||
await page.getByTestId("message-input").fill(`${firstUrl} ${secondUrl}`);
|
||||
await expect(page.locator("[data-composer-link-previews]")).toHaveCount(0);
|
||||
|
||||
Reference in New Issue
Block a user