From a64f3a6c4c5b67b3e36e7608f47ef1e8ec1ab08f Mon Sep 17 00:00:00 2001 From: sua yoo Date: Wed, 15 Jan 2025 22:58:32 -0800 Subject: [PATCH] fix: Fully load thumbnail before save (#2307) Fixes https://github.com/webrecorder/browsertrix/issues/2306 ## Changes Refactors collection view configuration to wait for thumbnail preview image (using `URL.createObjectURL`, like in QA screenshots) to be fully loaded from `replay-web-page` before saving. --- .../collections/collection-replay-dialog.ts | 106 ++++------- .../collection-snapshot-preview.ts | 172 ++++++++++++++++++ .../features/collections/share-collection.ts | 2 +- frontend/src/pages/org/collection-detail.ts | 14 +- 4 files changed, 220 insertions(+), 74 deletions(-) create mode 100644 frontend/src/features/collections/collection-snapshot-preview.ts diff --git a/frontend/src/features/collections/collection-replay-dialog.ts b/frontend/src/features/collections/collection-replay-dialog.ts index 925869e5..c2bc246b 100644 --- a/frontend/src/features/collections/collection-replay-dialog.ts +++ b/frontend/src/features/collections/collection-replay-dialog.ts @@ -6,16 +6,14 @@ import { customElement, property, query, state } from "lit/decorators.js"; import { when } from "lit/directives/when.js"; import queryString from "query-string"; +import { + HomeView, + type CollectionSnapshotPreview, +} from "./collection-snapshot-preview"; import type { SelectSnapshotDetail } from "./select-collection-start-page"; import { BtrixElement } from "@/classes/BtrixElement"; import type { Dialog } from "@/components/ui/dialog"; -import { formatRwpTimestamp } from "@/utils/replay"; - -enum HomeView { - Pages = "pages", - URL = "url", -} /** * @fires btrix-change @@ -76,11 +74,11 @@ export class CollectionStartPageDialog extends BtrixElement { private readonly form?: HTMLFormElement | null; @query("#thumbnailPreview") - private readonly thumbnailPreview?: HTMLIFrameElement | null; + private readonly thumbnailPreview?: CollectionSnapshotPreview | null; willUpdate(changedProperties: PropertyValues) { - if (changedProperties.has("homeUrl") && this.homeUrl) { - this.homeView = HomeView.URL; + if (changedProperties.has("homeUrl")) { + this.homeView = this.homeUrl ? HomeView.URL : HomeView.Pages; } } @@ -89,7 +87,7 @@ export class CollectionStartPageDialog extends BtrixElement { this.homeView === HomeView.URL && !this.selectedSnapshot; return html` (this.showContent = true)} @@ -142,11 +140,6 @@ export class CollectionStartPageDialog extends BtrixElement { } private renderPreview() { - let urlPreview = html` -

- ${msg("Enter a Page URL to preview it")} -

- `; const snapshot = this.selectedSnapshot || (this.homeUrl @@ -154,22 +147,19 @@ export class CollectionStartPageDialog extends BtrixElement { url: this.homeUrl, ts: this.homeTs, pageId: this.homePageId, + status: 200, } : null); - if (snapshot) { - urlPreview = html` - - - ${snapshot.url} - - `; - } + const replaySource = `/api/orgs/${this.orgId}/collections/${this.collectionId}/replay.json`; + // TODO Get query from replay-web-page embed + const query = queryString.stringify({ + source: replaySource, + customColl: this.collectionId, + embed: "default", + noCache: 1, + noSandbox: 1, + }); return html`
- ${when( - this.homeView === HomeView.URL && this.replayLoaded, - () => urlPreview, - )} -
- ${this.renderReplay()} -
+ + ${when( !this.replayLoaded, @@ -281,34 +273,18 @@ export class CollectionStartPageDialog extends BtrixElement { `; } - private renderReplay() { - const replaySource = `/api/orgs/${this.orgId}/collections/${this.collectionId}/replay.json`; - // TODO Get query from replay-web-page embed - const query = queryString.stringify({ - source: replaySource, - customColl: this.collectionId, - embed: "default", - noCache: 1, - noSandbox: 1, - }); - - return html`
-
- -
-
`; - } - private async onSubmit(e: SubmitEvent) { e.preventDefault(); const form = e.currentTarget as HTMLFormElement; const { homeView, useThumbnail } = serialize(form); - if (homeView === HomeView.Pages && !this.homePageId) { + if ( + (homeView === HomeView.Pages && !this.homePageId) || + (homeView === HomeView.URL && + this.selectedSnapshot && + this.homePageId === this.selectedSnapshot.pageId) + ) { // No changes to save this.open = false; return; @@ -322,8 +298,6 @@ export class CollectionStartPageDialog extends BtrixElement { (homeView === HomeView.URL && this.selectedSnapshot?.pageId) || null, }); - this.dispatchEvent(new CustomEvent("btrix-change")); - const shouldUpload = homeView === HomeView.URL && useThumbnail === "on" && @@ -333,19 +307,13 @@ export class CollectionStartPageDialog extends BtrixElement { const fileName = `page-thumbnail_${this.selectedSnapshot?.pageId}.jpeg`; let file: File | undefined; - if (shouldUpload && this.thumbnailPreview?.src) { - const { src } = this.thumbnailPreview; - - // Wait to get the thumbnail image before closing the dialog - try { - const resp = await this.thumbnailPreview.contentWindow!.fetch(src); - const blob = await resp.blob(); + if (shouldUpload && this.thumbnailPreview) { + const blob = await this.thumbnailPreview.thumbnailBlob; + if (blob) { file = new File([blob], fileName, { type: blob.type, }); - } catch (err) { - console.debug(err); } } else { this.notify.toast({ @@ -387,6 +355,8 @@ export class CollectionStartPageDialog extends BtrixElement { }); } } + + this.dispatchEvent(new CustomEvent("btrix-change")); } catch (err) { console.debug(err); diff --git a/frontend/src/features/collections/collection-snapshot-preview.ts b/frontend/src/features/collections/collection-snapshot-preview.ts new file mode 100644 index 00000000..b335149d --- /dev/null +++ b/frontend/src/features/collections/collection-snapshot-preview.ts @@ -0,0 +1,172 @@ +import { localized, msg } from "@lit/localize"; +import { Task } from "@lit/task"; +import clsx from "clsx"; +import { html, type PropertyValues } from "lit"; +import { customElement, property, query, state } from "lit/decorators.js"; + +import type { SelectSnapshotDetail } from "./select-collection-start-page"; + +import { TailwindElement } from "@/classes/TailwindElement"; +import { formatRwpTimestamp } from "@/utils/replay"; +import { tw } from "@/utils/tailwind"; + +export enum HomeView { + Pages = "pages", + URL = "url", +} + +/** + * Display preview of page snapshot. + * + * A previously loaded `replay-web-page` embed is required in order for preview to work. + */ +@customElement("btrix-collection-snapshot-preview") +@localized() +export class CollectionSnapshotPreview extends TailwindElement { + @property({ type: String }) + collectionId = ""; + + @property({ type: String }) + replaySrc = ""; + + @property({ type: String }) + view?: HomeView; + + @property({ type: Object }) + snapshot?: SelectSnapshotDetail["item"]; + + @query("iframe") + private readonly iframe?: HTMLIFrameElement | null; + + @state() + private iframeLoaded = false; + + public get thumbnailBlob() { + return this.blobTask.taskComplete.finally(() => this.blobTask.value); + } + + private readonly blobTask = new Task(this, { + task: async ([collectionId, snapshot, iframeLoaded]) => { + if ( + !collectionId || + !snapshot || + !iframeLoaded || + !this.iframe?.contentWindow + ) { + return; + } + + const resp = await this.iframe.contentWindow.fetch( + `/replay/w/${this.collectionId}/${formatRwpTimestamp(snapshot.ts)}id_/urn:thumbnail:${snapshot.url}`, + ); + + if (resp.status === 200) { + return await resp.blob(); + } + + throw new Error(`couldn't get thumbnail`); + }, + args: () => [this.collectionId, this.snapshot, this.iframeLoaded] as const, + }); + + private readonly objectUrlTask = new Task(this, { + task: ([blob]) => { + if (!blob) return ""; + + const url = URL.createObjectURL(blob); + + if (url) return url; + + throw new Error("no object url"); + }, + args: () => [this.blobTask.value] as const, + }); + + disconnectedCallback(): void { + super.disconnectedCallback(); + + if (this.objectUrlTask.value) { + URL.revokeObjectURL(this.objectUrlTask.value); + } + } + + protected willUpdate(changedProperties: PropertyValues): void { + if ( + changedProperties.has("collectionId") || + changedProperties.has("snapshot") + ) { + if (this.objectUrlTask.value) { + URL.revokeObjectURL(this.objectUrlTask.value); + } + } + } + + render() { + return html` ${this.renderSnapshot()} ${this.renderReplay()} `; + } + + private renderSnapshot() { + if (this.view === HomeView.Pages) return; + + return this.blobTask.render({ + complete: this.renderImage, + pending: this.renderSpinner, + error: this.renderError, + }); + } + + private readonly renderImage = () => { + if (!this.snapshot) { + return html` +

+ ${msg("Enter a Page URL to preview it")} +

+ `; + } + + return html` +
+ + ${this.objectUrlTask.render({ + complete: (value) => + value + ? html`` + : this.renderSpinner(), + pending: () => "pending", + })} + ${this.snapshot.url} + +
+ `; + }; + + private renderReplay() { + return html`
+
+
+ +
+
+
`; + } + + private readonly renderError = () => html` +

+ ${msg("Couldn't load preview. Try another snapshot")} +

+ `; + + private readonly renderSpinner = () => html` +
+ +
+ `; +} diff --git a/frontend/src/features/collections/share-collection.ts b/frontend/src/features/collections/share-collection.ts index e1d57126..1e97db06 100644 --- a/frontend/src/features/collections/share-collection.ts +++ b/frontend/src/features/collections/share-collection.ts @@ -246,7 +246,7 @@ export class ShareCollection extends BtrixElement { @sl-after-hide=${() => { this.tabGroup?.show(Tab.Link); }} - class="[--width:40rem] [--body-spacing:0]" + class="[--body-spacing:0] [--width:40rem]" > (this.openDialogName = "editStartPage")} - ?disabled=${!this.collection?.crawlCount} + ?disabled=${!this.collection?.crawlCount || + !this.isRwpLoaded} > - - ${msg("Configure Home")} + ${!this.collection || + Boolean(this.collection.crawlCount && !this.isRwpLoaded) + ? html`` + : html``} + ${msg("Configure View")} `, @@ -400,8 +404,8 @@ export class CollectionDetail extends BtrixElement { }} ?disabled=${!this.collection?.crawlCount} > - - ${msg("Configure Replay Home")} + + ${msg("Configure Replay View")} {