mirror of
https://github.com/TriliumNext/Trilium.git
synced 2026-10-11 22:49:56 +00:00
perf(clipper): read the whole page only when it is needed
The previous commit parsed every page twice with Defuddle, once as an
article and once as a whole, so that the Article / Whole page switch
was instant. Most clips never use the switch, so every popup and every
shortcut save paid for a parse whose result was thrown away.
`readable-page.js` now reads one way per call. A file injection takes
no arguments, so the script only registers
`globalThis.triliumReadPage(variant)`, and `extractPage(tabId, variant)`
calls it with a second `executeScript({ func, args })` in the same
isolated world. Two script files would have bundled Defuddle twice.
- The popup reads the article, and the whole page only when there is
no article. Switching to Whole page reads it then, showing "Reading
the page…" with Save disabled, and `PagePreview` keeps the result in
`laterReadings` so switching back and forth reads it once. A reading
is `undefined` until made and `null` when it shows nothing or fails,
which disables its button and falls back to the article.
- The shortcut and the context menu read the article, and the whole
page only when there is none: one parse on an article page, as
before the switch existed.
`readActivePage()` falls back with consecutive `if`s rather than
`article ?? await extractPage(...)`: written with `??`, v8 recorded a
negative count for the code after it, and the branch gate failed with
every case tested.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
Elian Doran
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
9e35303f64
commit
442f2a07ca
@@ -18,7 +18,7 @@ repository.
|
||||
| `entrypoints/background/index.ts` | service worker (MV3) / background page (MV2) | context menus, keyboard commands, every save, image download, toasts |
|
||||
| `entrypoints/background/trilium_server_facade.ts` | background | finds Trilium, authenticates, `callService()` |
|
||||
| `entrypoints/content/index.ts` | every page (`<all_urls>`) | selection, page description, crop overlay, toast UI |
|
||||
| `entrypoints/readable-page/index.ts` | the tab, injected on demand | Defuddle extraction of the page, as an article and as a whole; its `main()` value is the `executeScript()` result |
|
||||
| `entrypoints/readable-page/index.ts` | the tab, injected on demand | registers `triliumReadPage()`: Defuddle extraction of the page, as an article or as a whole |
|
||||
| `entrypoints/popup/main.tsx` | the toolbar popup | Preact `Popup`: capture buttons, link-with-note form, connection status |
|
||||
| `entrypoints/options/main.tsx` | the options page | Preact `Options`: desktop port, server login |
|
||||
| `entrypoints/offscreen/` | MV3 only (Chrome) | crops screenshots on a canvas, since an MV3 service worker has no DOM |
|
||||
@@ -38,11 +38,14 @@ Everything goes through `browser.runtime.sendMessage` / `browser.tabs.sendMessag
|
||||
(`message`, `noteId`, `tabIds`).
|
||||
- **background → offscreen:** `{ type: "CROP_IMAGE" }`, the one message keyed by `type`.
|
||||
|
||||
The readable page is not a message: the popup and the background call `extractPage(tabId)`
|
||||
(`utils.ts`), which injects `/readable-page.js` with `browser.scripting.executeScript()` (the
|
||||
`scripting` permission, in both builds), so Defuddle stays out of the content script that loads
|
||||
on every page. Specs mock it with `Object.assign(fakeBrowser, { scripting: { executeScript } })`,
|
||||
answering `[ { result: { article, wholePage } } ]` (`ExtractedPage`).
|
||||
The readable page is not a message: the popup and the background call
|
||||
`extractPage(tabId, variant)` (`utils.ts`), which injects `/readable-page.js` with
|
||||
`browser.scripting.executeScript()` (the `scripting` permission, in both builds), so Defuddle stays
|
||||
out of the content script that loads on every page. A file injection takes no arguments, so the
|
||||
script only registers `globalThis.triliumReadPage()` and a second `executeScript({ func, args })`
|
||||
calls it in the same isolated world. Specs mock it with
|
||||
`Object.assign(fakeBrowser, { scripting: { executeScript } })`, answering `[]` to the file and
|
||||
`[ { result: page } ]` to the call, by `args[0]`.
|
||||
|
||||
Every user action in the background runs inside `showFailures()`, which turns an exception into a
|
||||
toast. A new action belongs inside it too, or it fails silently. A toast cannot show on browser
|
||||
@@ -120,16 +123,20 @@ against older versions of the other.** Prefer changes that need nothing new from
|
||||
made with `cloneNode()` does not have. `url` makes links and images absolute.
|
||||
- Its HTML is parsed with `DOMParser`, a document that loads no image and runs no code, before
|
||||
`getImages()` swaps the sources for placeholders.
|
||||
- `readPage()` parses twice: the article, and the whole page (`WHOLE_PAGE_OPTIONS`:
|
||||
- A page reads two ways: `"article"`, and `"wholePage"` (`WHOLE_PAGE_OPTIONS`:
|
||||
`contentSelector: "body"` with scoring and the clutter selectors off, hidden-element removal
|
||||
and standardization kept). Defuddle drops a grid of cards with little text under each heading
|
||||
as "related articles" (`isCardGrid()`) and any `.footer`, which on a gallery or test page is the
|
||||
content (#9611). The popup offers both (`Article` / `Whole page`); the shortcut and the menu
|
||||
save the article, or the whole page when there is none.
|
||||
- A page without an article comes back as `<body></body>`, which `readPage()` turns into
|
||||
`article: undefined`; only a page whose whole page is empty too returns `undefined` (the
|
||||
popup's bookmark mode). Defuddle's own retries relax its rules on a very short page ("possible
|
||||
index page"), so a spec fixture of a card grid needs a paragraph of text to stay above that.
|
||||
content (#9611). Each reading is one parse, made only when needed: the popup and the shortcut
|
||||
read the article, and the whole page only when there is no article — or, in the popup, when
|
||||
the user switches to `Whole page` (`laterReadings` in `PagePreview`).
|
||||
- A reading that shows nothing (an article-less page comes back as `<body></body>`) is
|
||||
`undefined`; the popup stores it as `null`, keeping `undefined` for "not read yet". A page
|
||||
whose two readings both show nothing opens on Bookmark. Defuddle's own retries relax its rules
|
||||
on a very short page ("possible index page"), so a spec fixture of a card grid needs a
|
||||
paragraph of text to stay above that.
|
||||
- v8 can record a negative count for the code after `x ?? await y()`, which fails the branch
|
||||
gate with every case tested; write such a fallback as consecutive `if`s.
|
||||
- Use `parse()`, never `parseAsync()`: the async extractors fetch third-party APIs.
|
||||
- **happy-dom drops lazy images** (`data-src` behind a `data:image/svg+xml` placeholder) that
|
||||
Chromium and linkedom promote to `src`; test such Defuddle behavior in a real browser, not
|
||||
|
||||
@@ -56,9 +56,9 @@ let tabs: Tab[];
|
||||
let tabMessageHandler: (message: { name: string }) => unknown;
|
||||
const tabsSendMessage = vi.fn(
|
||||
(_tabId: number, message: { name: string }) => Promise.resolve(tabMessageHandler(message)));
|
||||
/** What `readable-page.js` returns in the active tab; nothing to save unless a test says so. */
|
||||
let extractedPage: unknown;
|
||||
const executeScript = vi.fn(async () => [ { result: extractedPage } ]);
|
||||
/** How `readable-page.js` reads the active tab, by variant; nothing to save unless a test says so. */
|
||||
let pageReadings: Record<string, unknown>;
|
||||
const executeScript = vi.fn(async ({ args }: { args?: [ string ] }) => args ? [ { result: pageReadings[args[0]] } ] : []);
|
||||
const contextMenusCreate = vi.fn();
|
||||
const contextMenusRemoveAll = vi.fn(async () => {});
|
||||
const getContexts = vi.fn(async (): Promise<unknown[]> => []);
|
||||
@@ -76,7 +76,7 @@ describe("background", () => {
|
||||
fakeBrowser.reset();
|
||||
tabs = [ ACTIVE_TAB ];
|
||||
tabMessageHandler = () => undefined;
|
||||
extractedPage = undefined;
|
||||
pageReadings = {};
|
||||
facade.callService.mockResolvedValue({ noteId: "saved" });
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
@@ -283,14 +283,15 @@ describe("background", () => {
|
||||
describe("saving the whole page", () => {
|
||||
it("saves the article from the shortcut, the menu and the popup", async () => {
|
||||
const page = { title: "Article", content: "<p>Body</p>", images: [], clipType: "page" };
|
||||
extractedPage = { article: page, wholePage: { ...page, content: "<nav>Menu</nav><p>Body</p>" } };
|
||||
pageReadings = { article: page, wholePage: { ...page, content: "<nav>Menu</nav><p>Body</p>" } };
|
||||
|
||||
await onCommand("saveWholePage");
|
||||
await onContextMenuClicked({ menuItemId: "trilium-save-page" });
|
||||
await sendRuntimeMessage({ name: "save-whole-page" });
|
||||
|
||||
expect(executeScript).toHaveBeenCalledTimes(3);
|
||||
expect(executeScript).toHaveBeenCalledTimes(6);
|
||||
expect(executeScript).toHaveBeenCalledWith({ target: { tabId: 7 }, files: [ "/readable-page.js" ] });
|
||||
expect(executeScript).not.toHaveBeenCalledWith(expect.objectContaining({ args: [ "wholePage" ] }));
|
||||
expect(facade.callService).toHaveBeenCalledTimes(3);
|
||||
expect(facade.callService).toHaveBeenLastCalledWith("POST", "notes", page);
|
||||
expect(await lastToast()).toMatchObject({ noteId: "saved" });
|
||||
@@ -303,7 +304,7 @@ describe("background", () => {
|
||||
|
||||
it("saves the whole page from the shortcut when the page has no article", async () => {
|
||||
const wholePage = { title: "Gallery", content: "<h2>Card</h2>", images: [], clipType: "page" };
|
||||
extractedPage = { wholePage };
|
||||
pageReadings = { wholePage };
|
||||
|
||||
await onCommand("saveWholePage");
|
||||
|
||||
@@ -318,7 +319,7 @@ describe("background", () => {
|
||||
message: "Saving to Trilium failed: Trilium found nothing to save on this page."
|
||||
});
|
||||
|
||||
extractedPage = { wholePage: { title: "Article", content: "<p>Body</p>", images: [] } };
|
||||
pageReadings = { wholePage: { title: "Article", content: "<p>Body</p>", images: [] } };
|
||||
tabs = [ { title: "Tab without id" } ];
|
||||
await onCommand("saveWholePage");
|
||||
|
||||
@@ -330,7 +331,7 @@ describe("background", () => {
|
||||
message: "Saving to Trilium failed: Cannot access a chrome:// URL"
|
||||
});
|
||||
|
||||
expect(executeScript).toHaveBeenCalledTimes(2);
|
||||
expect(executeScript).toHaveBeenCalledTimes(5);
|
||||
expect(facade.callService).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
@@ -347,11 +347,14 @@ export default defineBackground(() => {
|
||||
|
||||
async function readActivePage() {
|
||||
const activeTab = await getActiveTab();
|
||||
const page = activeTab.id === undefined ? undefined : await extractPage(activeTab.id);
|
||||
if (!page) {
|
||||
throw new Error("Trilium found nothing to save on this page.");
|
||||
const tabId = activeTab.id;
|
||||
if (tabId !== undefined) {
|
||||
const article = await extractPage(tabId, "article");
|
||||
if (article) return article;
|
||||
const wholePage = await extractPage(tabId, "wholePage");
|
||||
if (wholePage) return wholePage;
|
||||
}
|
||||
return page.article ?? page.wholePage;
|
||||
throw new Error("Trilium found nothing to save on this page.");
|
||||
}
|
||||
|
||||
async function saveLinkWithNote(title: string, content: string, description?: string) {
|
||||
|
||||
@@ -24,7 +24,7 @@ import type { ComponentChildren } from "preact";
|
||||
import { Fragment, render } from "preact";
|
||||
import { useEffect, useRef, useState } from "preact/hooks";
|
||||
|
||||
import { type ExtractedPage, extractPage, type ReadablePage } from "@/utils";
|
||||
import { extractPage, type PageVariant, type ReadablePage } from "@/utils";
|
||||
|
||||
import type { TriliumSearchNoteStatus, TriliumSearchStatus } from "../background/trilium_server_facade";
|
||||
|
||||
@@ -52,8 +52,11 @@ interface SelectedContent {
|
||||
labels?: Record<string, string>;
|
||||
}
|
||||
|
||||
/** The current page: its article and its whole page, or why there is nothing to save. */
|
||||
type PageState = ExtractedPage | "unreadable" | "inaccessible";
|
||||
/** The tab, and each reading of its page: `undefined` until it is read, `null` when it shows nothing. */
|
||||
type PageReadings = { tabId: number } & Record<PageVariant, ReadablePage | null | undefined>;
|
||||
|
||||
/** The current page: its readings, or why there is nothing to save. */
|
||||
type PageState = PageReadings | "unreadable" | "inaccessible";
|
||||
|
||||
/** What the active tab offers to save. */
|
||||
interface TabContent {
|
||||
@@ -73,9 +76,6 @@ interface TabContent {
|
||||
*/
|
||||
type ClipMode = "selection" | "page" | "note";
|
||||
|
||||
/** Which reading of the page the `page` mode saves. */
|
||||
type PageVariant = keyof ExtractedPage;
|
||||
|
||||
/** What the user has chosen and typed in the card, kept by `Popup` so a reconnection does not lose it. */
|
||||
interface Draft {
|
||||
mode?: ClipMode;
|
||||
@@ -130,6 +130,12 @@ const MODE_LABELS: Record<ClipMode, string> = { selection: "Selection", page: "P
|
||||
|
||||
const VARIANT_LABELS: Record<PageVariant, string> = { article: "Article", wholePage: "Whole page" };
|
||||
|
||||
/** Why a reading of the page is unavailable. */
|
||||
const VARIANT_UNAVAILABLE: Record<PageVariant, string> = {
|
||||
article: "Trilium found no article on this page.",
|
||||
wholePage: "Trilium found nothing to save on this page."
|
||||
};
|
||||
|
||||
/** The save button of each mode, with the command whose shortcut it shows, or its own shortcut. */
|
||||
const SAVE_BUTTONS: Record<ClipMode, { icon: IconDefinition, label: string, command: string, shortcut?: string }> = {
|
||||
selection: { icon: Highlight, label: "Save selection", command: "saveSelection" },
|
||||
@@ -347,20 +353,33 @@ function PagePreview({ tab, disabled, shortcuts, draft, onDraftChange }: {
|
||||
}) {
|
||||
const { mode: chosenMode, variant: chosenVariant, title, description, author, text } = draft;
|
||||
const textAreaRef = useRef<HTMLTextAreaElement>(null);
|
||||
/** The readings of the page made after the popup opened, when the user switched to them. */
|
||||
const [ laterReadings, setLaterReadings ] = useState<Partial<PageReadings>>({});
|
||||
|
||||
const page = typeof tab?.page === "object" ? tab.page : undefined;
|
||||
const variant = chosenVariant && page?.[chosenVariant] ? chosenVariant : page?.article ? "article" : "wholePage";
|
||||
const pageClip = page?.[variant];
|
||||
const page = typeof tab?.page === "object" ? { ...tab.page, ...laterReadings } : undefined;
|
||||
const variant = chosenVariant && page?.[chosenVariant] !== null ? chosenVariant : page?.article ? "article" : "wholePage";
|
||||
const pageClip = page?.[variant] ?? undefined;
|
||||
/** Either reading: both carry the same title and labels. */
|
||||
const pageReading = page?.article ?? page?.wholePage ?? undefined;
|
||||
const selection = tab?.selection ?? undefined;
|
||||
const modes = tab ? availableModes(pageClip, selection) : [];
|
||||
const modes = tab ? availableModes(!!page, selection) : [];
|
||||
const mode = chosenMode && modes.includes(chosenMode) ? chosenMode : modes[0];
|
||||
const clip = mode === "selection" ? selection : mode === "page" ? pageClip : undefined;
|
||||
const shownTitle = title ?? (pageClip ?? selection)?.title ?? tab?.title ?? "";
|
||||
const shownTitle = title ?? (pageReading ?? selection)?.title ?? tab?.title ?? "";
|
||||
const shownDescription = description ?? tab?.description ?? "";
|
||||
const savedDescription = shownDescription.replace(/\s+/g, " ").trim();
|
||||
const shownAuthor = author ?? pageClip?.labels.pageAuthor ?? "";
|
||||
const shownAuthor = author ?? pageReading?.labels.pageAuthor ?? "";
|
||||
const savedAuthor = shownAuthor.replace(/\s+/g, " ").trim();
|
||||
|
||||
const tabId = page?.tabId;
|
||||
const unreadVariant = mode === "page" && page?.[variant] === undefined ? variant : undefined;
|
||||
useEffect(() => {
|
||||
if (tabId === undefined || !unreadVariant) return;
|
||||
void readVariant(tabId, unreadVariant).then((reading) => {
|
||||
setLaterReadings((readings) => ({ ...readings, [unreadVariant]: reading }));
|
||||
});
|
||||
}, [ tabId, unreadVariant ]);
|
||||
|
||||
useEffect(() => {
|
||||
if (mode === "note") textAreaRef.current?.focus();
|
||||
}, [ mode ]);
|
||||
@@ -404,7 +423,7 @@ function PagePreview({ tab, disabled, shortcuts, draft, onDraftChange }: {
|
||||
}
|
||||
|
||||
const pageUrl = mode === "note" ? tab?.url : clip?.pageUrl;
|
||||
const pageLabels = mode === "page" ? pageClip?.labels : undefined;
|
||||
const pageLabels = mode === "page" ? pageReading?.labels : undefined;
|
||||
const published = pageLabels?.publishedDate;
|
||||
const modified = pageLabels?.modifiedDate;
|
||||
const button = SAVE_BUTTONS[mode ?? "page"];
|
||||
@@ -521,8 +540,8 @@ function PagePreview({ tab, disabled, shortcuts, draft, onDraftChange }: {
|
||||
<button
|
||||
key={option}
|
||||
aria-pressed={option === variant}
|
||||
disabled={!page[option]}
|
||||
title={page[option] ? undefined : "Trilium found no article on this page."}
|
||||
disabled={page[option] === null}
|
||||
title={page[option] === null ? VARIANT_UNAVAILABLE[option] : undefined}
|
||||
onClick={() => onDraftChange({ ...draft, variant: option })}
|
||||
>
|
||||
{VARIANT_LABELS[option]}
|
||||
@@ -539,12 +558,12 @@ function PagePreview({ tab, disabled, shortcuts, draft, onDraftChange }: {
|
||||
onLoad={(e) => fitToContent(e.currentTarget)}
|
||||
/>
|
||||
)}
|
||||
{!mode && <div className="page-preview-placeholder">Reading the page…</div>}
|
||||
{(!mode || (mode === "page" && !clip)) && <div className="page-preview-placeholder">Reading the page…</div>}
|
||||
</div>
|
||||
|
||||
<button
|
||||
className="btn btn-primary primary-action"
|
||||
disabled={disabled || !mode}
|
||||
disabled={disabled || !onSave}
|
||||
title={disabled ? DISCONNECTED_TITLE : undefined}
|
||||
onClick={onSave}
|
||||
>
|
||||
@@ -580,10 +599,10 @@ function fitToContent(frame: HTMLIFrameElement) {
|
||||
}
|
||||
|
||||
/** The modes the page offers, the default first: the selection, then the page, then a note. */
|
||||
function availableModes(page: ReadablePage | undefined, selection: SelectedContent | undefined): ClipMode[] {
|
||||
function availableModes(hasPage: boolean, selection: SelectedContent | undefined): ClipMode[] {
|
||||
const modes: ClipMode[] = [];
|
||||
if (selection) modes.push("selection");
|
||||
if (page) modes.push("page");
|
||||
if (hasPage) modes.push("page");
|
||||
modes.push("note");
|
||||
return modes;
|
||||
}
|
||||
@@ -651,17 +670,28 @@ async function readActiveTab(): Promise<TabContent> {
|
||||
}
|
||||
|
||||
/**
|
||||
* `extractPage()` answers `undefined` when the page shows nothing, and fails where the extension
|
||||
* cannot run scripts.
|
||||
* Reads the article, and the whole page only when there is no article: the user can ask for it
|
||||
* later. `extractPage()` fails where the extension cannot run scripts.
|
||||
*/
|
||||
async function readPage(tabId: number): Promise<PageState> {
|
||||
try {
|
||||
return await extractPage(tabId) ?? "unreadable";
|
||||
const article = await extractPage(tabId, "article") ?? null;
|
||||
const wholePage = article ? undefined : await extractPage(tabId, "wholePage") ?? null;
|
||||
return article || wholePage ? { tabId, article, wholePage } : "unreadable";
|
||||
} catch {
|
||||
return "inaccessible";
|
||||
}
|
||||
}
|
||||
|
||||
/** The page as `variant` reads it, or `null` when that reading shows nothing or fails. */
|
||||
async function readVariant(tabId: number, variant: PageVariant) {
|
||||
try {
|
||||
return await extractPage(tabId, variant) ?? null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The selection on the page, or `null` when there is none. A click on the page leaves a collapsed
|
||||
* selection, which the content script still answers with, empty.
|
||||
|
||||
@@ -28,8 +28,10 @@ const PAGE = {
|
||||
labels: { pageAuthor: "Jane Doe", publishedDate: "2024-05-01", modifiedDate: "2024-06-12" }
|
||||
};
|
||||
const WHOLE_PAGE = { ...PAGE, content: `<nav>Menu</nav><p>Body</p><img src="i1">` };
|
||||
/** What `readable-page.js` returns in the tab, as `browser.scripting.executeScript()` answers. */
|
||||
const executeScript = vi.fn(async (): Promise<unknown> => extracted(PAGE));
|
||||
/** How `readable-page.js` reads the tab, by variant, as `browser.scripting.executeScript()` answers. */
|
||||
let pageReadings: Record<string, unknown> = {};
|
||||
const executeScript = vi.fn(async ({ args }: { args?: [ string ] }): Promise<unknown> =>
|
||||
args ? [ { result: pageReadings[args[0]] } ] : []);
|
||||
const tabsQuery = vi.fn(async (): Promise<{ id?: number, title?: string, url?: string }[]> => [ { id: 7 } ]);
|
||||
const SELECTION = {
|
||||
title: "Page title",
|
||||
@@ -56,6 +58,7 @@ describe("popup", () => {
|
||||
fakeBrowser.reset();
|
||||
selection = undefined;
|
||||
metadata = {};
|
||||
pageReadings = { article: PAGE, wholePage: WHOLE_PAGE };
|
||||
Object.assign(fakeBrowser.runtime, { sendMessage, openOptionsPage });
|
||||
Object.assign(fakeBrowser.commands, { getAll: getAllCommands });
|
||||
Object.assign(fakeBrowser.tabs, { query: tabsQuery, sendMessage: tabsSendMessage });
|
||||
@@ -235,13 +238,30 @@ describe("popup", () => {
|
||||
expect(button("Save page to Trilium")?.disabled).toBe(false);
|
||||
});
|
||||
|
||||
it("switches the page between its article and the whole page, and keeps the choice", async () => {
|
||||
it("reads the whole page when the user switches to it, once, and keeps the choice", async () => {
|
||||
expect(variants()).toEqual([ [ "Article", "true", false ], [ "Whole page", "false", false ] ]);
|
||||
expect(container.querySelector(".page-variant")?.getAttribute("aria-label")).toBe("How to read the page");
|
||||
expect(wholePageReads()).toBe(0);
|
||||
|
||||
let finishReading: () => void = () => {};
|
||||
executeScript
|
||||
.mockResolvedValueOnce([])
|
||||
.mockReturnValueOnce(new Promise((resolve) => {
|
||||
finishReading = () => resolve([ { result: WHOLE_PAGE } ]);
|
||||
}));
|
||||
await click("Whole page");
|
||||
expect(variants()).toEqual([ [ "Article", "false", false ], [ "Whole page", "true", false ] ]);
|
||||
expect(placeholder()).toBe("Reading the page…");
|
||||
expect(button("Save page to Trilium")?.disabled).toBe(true);
|
||||
|
||||
await act(() => finishReading());
|
||||
await flush();
|
||||
expect(frameDocument()).toBe(previewDocument(WHOLE_PAGE));
|
||||
await click("Article");
|
||||
expect(frameDocument()).toBe(previewDocument(PAGE));
|
||||
await click("Whole page");
|
||||
expect(frameDocument()).toBe(previewDocument(WHOLE_PAGE));
|
||||
expect(wholePageReads()).toBe(1);
|
||||
|
||||
await click("Bookmark");
|
||||
expect(container.querySelector(".page-variant")).toBeNull();
|
||||
@@ -249,6 +269,7 @@ describe("popup", () => {
|
||||
await receive({ name: "trilium-search-status", triliumSearch: { status: "found-desktop", port: 37840, url: "" } });
|
||||
await flush();
|
||||
await click("Page");
|
||||
await flush();
|
||||
expect(variants()).toEqual([ [ "Article", "false", false ], [ "Whole page", "true", false ] ]);
|
||||
|
||||
await type(".page-title", "Gallery");
|
||||
@@ -259,8 +280,23 @@ describe("popup", () => {
|
||||
expect(frameDocument()).toBe(previewDocument(PAGE));
|
||||
});
|
||||
|
||||
it("goes back to the article when the whole page shows nothing or cannot be read", async () => {
|
||||
pageReadings = { article: PAGE };
|
||||
await click("Whole page");
|
||||
await flush();
|
||||
expect(variants()).toEqual([ [ "Article", "true", false ], [ "Whole page", "false", true ] ]);
|
||||
expect(button("Whole page")?.title).toBe("Trilium found nothing to save on this page.");
|
||||
expect(frameDocument()).toBe(previewDocument(PAGE));
|
||||
|
||||
await rerender();
|
||||
executeScript.mockRejectedValueOnce(new Error("The tab was closed."));
|
||||
await click("Whole page");
|
||||
await flush();
|
||||
expect(variants()).toEqual([ [ "Article", "true", false ], [ "Whole page", "false", true ] ]);
|
||||
});
|
||||
|
||||
it("opens a page without an article on the whole page", async () => {
|
||||
executeScript.mockResolvedValueOnce(extracted(undefined));
|
||||
pageReadings = { wholePage: WHOLE_PAGE };
|
||||
await rerender();
|
||||
expect(modes()).toEqual([ [ "Page", "true" ], [ "Bookmark", "false" ] ]);
|
||||
expect(variants()).toEqual([ [ "Article", "false", true ], [ "Whole page", "true", false ] ]);
|
||||
@@ -284,7 +320,7 @@ describe("popup", () => {
|
||||
});
|
||||
|
||||
it("leaves out the dates of a page that states none", async () => {
|
||||
executeScript.mockResolvedValueOnce(extracted({ ...PAGE, labels: {} }));
|
||||
pageReadings = { article: { ...PAGE, labels: {} }, wholePage: WHOLE_PAGE };
|
||||
await rerender();
|
||||
expect(container.querySelector(".page-meta")?.textContent).toBe("example.com");
|
||||
expect(container.querySelectorAll(".page-fields > .page-field")).toHaveLength(2);
|
||||
@@ -302,13 +338,13 @@ describe("popup", () => {
|
||||
expect(cardParts()).toEqual([ "page-body", "btn btn-primary primary-action" ]);
|
||||
expect(button("Save page to Trilium")?.disabled).toBe(true);
|
||||
|
||||
resolvePage(extracted(PAGE));
|
||||
resolvePage([]);
|
||||
await flush();
|
||||
expect(container.querySelector(".page-body iframe")).not.toBeNull();
|
||||
});
|
||||
|
||||
it("opens a page with nothing to save on a bookmark, and keeps every other action", async () => {
|
||||
executeScript.mockResolvedValueOnce([ { result: null } ]);
|
||||
pageReadings = {};
|
||||
await rerender();
|
||||
expect(container.querySelector(".no-items")).toBeNull();
|
||||
expect(container.querySelector(".clip-mode")).toBeNull();
|
||||
@@ -334,7 +370,7 @@ describe("popup", () => {
|
||||
tabsQuery.mockResolvedValueOnce([ {} ]);
|
||||
await rerender();
|
||||
expect(unavailable()).toContain("The extension cannot access this page.");
|
||||
expect(executeScript).toHaveBeenCalledTimes(2);
|
||||
expect(executeScript).toHaveBeenCalledTimes(3);
|
||||
expect(tabsSendMessage).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
@@ -365,7 +401,7 @@ describe("popup", () => {
|
||||
|
||||
it("saves the selection of a page with nothing else to save, and ignores an empty one", async () => {
|
||||
selection = SELECTION;
|
||||
executeScript.mockResolvedValueOnce([ { result: null } ]);
|
||||
pageReadings = {};
|
||||
await rerender();
|
||||
expect(container.querySelector(".no-items")).toBeNull();
|
||||
expect(modes()).toEqual([ [ "Selection", "true" ], [ "Bookmark", "false" ] ]);
|
||||
@@ -375,6 +411,7 @@ describe("popup", () => {
|
||||
expect(sendMessage).toHaveBeenLastCalledWith({ name: "save-selection", selection: SELECTION });
|
||||
|
||||
selection = { ...SELECTION, content: " <p> </p>", images: [] };
|
||||
pageReadings = { article: PAGE, wholePage: WHOLE_PAGE };
|
||||
await rerender();
|
||||
expect(modes()).toEqual([ [ "Page", "true" ], [ "Bookmark", "false" ] ]);
|
||||
expect(frameDocument()).toBe(previewDocument(PAGE));
|
||||
@@ -531,7 +568,7 @@ describe("popup", () => {
|
||||
expect(author()).toBeNull();
|
||||
|
||||
selection = undefined;
|
||||
executeScript.mockResolvedValueOnce(extracted({ ...PAGE, labels: {} }));
|
||||
pageReadings = { article: { ...PAGE, labels: {} }, wholePage: WHOLE_PAGE };
|
||||
await rerender();
|
||||
expect(author()?.value).toBe("");
|
||||
await type(".page-author", "Written by hand");
|
||||
@@ -838,16 +875,17 @@ function modes() {
|
||||
.map((option) => [ option.textContent, option.getAttribute("aria-pressed") ]);
|
||||
}
|
||||
|
||||
/** How many times the popup read the whole page. */
|
||||
function wholePageReads() {
|
||||
return executeScript.mock.calls.filter(([ injection ]) => injection.args?.[0] === "wholePage").length;
|
||||
}
|
||||
|
||||
/** The page variant switch's options, each with whether it is the current one and whether it is disabled. */
|
||||
function variants() {
|
||||
return [ ...container.querySelectorAll<HTMLButtonElement>(".page-body > .page-variant button") ]
|
||||
.map((option) => [ option.textContent, option.getAttribute("aria-pressed"), option.disabled ]);
|
||||
}
|
||||
|
||||
/** What `executeScript()` answers for a page with `article`, whose whole page is `WHOLE_PAGE`. */
|
||||
function extracted(article: object | undefined) {
|
||||
return [ { result: { article, wholePage: WHOLE_PAGE } } ];
|
||||
}
|
||||
|
||||
function frameDocument() {
|
||||
const frame = container.querySelector(".page-body iframe.page-content");
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { Window as HappyDOMWindow } from "happy-dom";
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
|
||||
import type { ExtractedPage } from "@/utils";
|
||||
import type { PageVariant } from "@/utils";
|
||||
|
||||
import readablePage from "./index";
|
||||
|
||||
@@ -37,7 +37,7 @@ describe("readable page", () => {
|
||||
</article>`;
|
||||
const pageBefore = document.documentElement.outerHTML;
|
||||
|
||||
const page = readPage()?.article;
|
||||
const page = readPage("article");
|
||||
|
||||
expect(page).toMatchObject({
|
||||
title: "Article title",
|
||||
@@ -74,12 +74,12 @@ describe("readable page", () => {
|
||||
document.head.innerHTML = `<title>Post title</title>${structuredData("2025-12-24T08:00:00Z")}
|
||||
<meta property="article:published_time" content="">`;
|
||||
document.body.innerHTML = article;
|
||||
expect(readPage()?.article?.labels).toEqual({ publishedDate: "2025-12-24" });
|
||||
expect(readPage("article")?.labels).toEqual({ publishedDate: "2025-12-24" });
|
||||
|
||||
document.head.innerHTML = `<title>Post title</title>${structuredData("next week")}
|
||||
<meta property="article:modified_time" content="not a date">`;
|
||||
document.body.innerHTML = article;
|
||||
expect(readPage()?.article?.labels).toEqual({});
|
||||
expect(readPage("article")?.labels).toEqual({});
|
||||
});
|
||||
|
||||
it("keeps the day the page states, whatever its offset from UTC", () => {
|
||||
@@ -88,25 +88,25 @@ describe("readable page", () => {
|
||||
<meta property="article:published_time" content="2022-02-09T00:30:00+01:00">
|
||||
<meta property="article:modified_time" content="2022-02-10T23:30:00-05:00">`;
|
||||
document.body.innerHTML = `<article><p>${PARAGRAPH.repeat(5)}</p></article>`;
|
||||
expect(readPage()?.article?.labels).toEqual({ publishedDate: "2022-02-09", modifiedDate: "2022-02-10" });
|
||||
expect(readPage("article")?.labels).toEqual({ publishedDate: "2022-02-09", modifiedDate: "2022-02-10" });
|
||||
|
||||
document.head.innerHTML = "<title>Post title</title>";
|
||||
document.body.innerHTML = `<article><p>${PARAGRAPH.repeat(5)}</p>
|
||||
<p>Published <time>February 8, 2022</time></p></article>`;
|
||||
expect(readPage()?.article?.labels).toEqual({});
|
||||
expect(readPage("article")?.labels).toEqual({});
|
||||
});
|
||||
|
||||
it("keeps an article that is only a picture, and the whole page of a page without one", () => {
|
||||
document.body.innerHTML = `<article><img src="photo.jpg" width="800" height="600"></article>`;
|
||||
expect(readPage()?.article?.images.map(({ src }) => src)).toEqual([ "https://example.com/blog/photo.jpg" ]);
|
||||
expect(readPage("article")?.images.map(({ src }) => src)).toEqual([ "https://example.com/blog/photo.jpg" ]);
|
||||
|
||||
document.body.innerHTML = "<nav><a href=\"/\">Home</a> <a href=\"/archive\">Archive</a></nav>";
|
||||
const page = readPage();
|
||||
expect(page?.article).toBeUndefined();
|
||||
expect(page?.wholePage.content).toContain("href=\"https://example.com/archive\"");
|
||||
expect(readPage("article")).toBeUndefined();
|
||||
expect(readPage("wholePage")?.content).toContain("href=\"https://example.com/archive\"");
|
||||
|
||||
document.body.innerHTML = "<div> </div>";
|
||||
expect(readPage()).toBeUndefined();
|
||||
expect(readPage("article")).toBeUndefined();
|
||||
expect(readPage("wholePage")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("keeps every card of a card grid in the whole page, without hidden elements", () => {
|
||||
@@ -132,7 +132,8 @@ describe("readable page", () => {
|
||||
<div style="display: none">Cookie banner</div>
|
||||
<script>window.tracked = true;</script>`;
|
||||
|
||||
const { article, wholePage } = readPage() ?? {};
|
||||
const article = readPage("article");
|
||||
const wholePage = readPage("wholePage");
|
||||
|
||||
expect(article?.images.length).toBeLessThan(4);
|
||||
expect(article?.content).not.toContain("What to check");
|
||||
@@ -151,6 +152,7 @@ describe("readable page", () => {
|
||||
});
|
||||
});
|
||||
|
||||
function readPage() {
|
||||
return readablePage.main() as ExtractedPage | undefined;
|
||||
function readPage(variant: PageVariant) {
|
||||
readablePage.main();
|
||||
return globalThis.triliumReadPage(variant);
|
||||
}
|
||||
|
||||
@@ -1,13 +1,15 @@
|
||||
import Defuddle from "defuddle";
|
||||
|
||||
import { descriptionLabels, type ExtractedPage, getImages, getPageLocationOrigin, type ReadablePage } from "@/utils";
|
||||
import { descriptionLabels, getImages, getPageLocationOrigin, type PageVariant, type ReadablePage } from "@/utils";
|
||||
|
||||
/**
|
||||
* Reads the page with Defuddle, as an article and as a whole. `extractPage()` injects it into the
|
||||
* tab only when a page is read, so Defuddle does not load with the content script on every page. The
|
||||
* value `main()` returns is the result of `executeScript()`.
|
||||
* Registers `triliumReadPage()` in the tab, which reads the page with Defuddle, as an article or as a
|
||||
* whole. `extractPage()` injects it into the tab only when a page is read, so Defuddle does not load
|
||||
* with the content script on every page, then calls it with the reading it needs.
|
||||
*/
|
||||
export default defineUnlistedScript(readPage);
|
||||
export default defineUnlistedScript(() => {
|
||||
globalThis.triliumReadPage = readPage;
|
||||
});
|
||||
|
||||
/**
|
||||
* Keeps all of `<body>`: Defuddle still removes hidden elements, makes links absolute and converts
|
||||
@@ -22,35 +24,28 @@ const WHOLE_PAGE_OPTIONS = {
|
||||
removeSmallImages: false
|
||||
};
|
||||
|
||||
function readPage(): ExtractedPage | undefined {
|
||||
/** The page as `variant` reads it, or `undefined` when that reading shows nothing. */
|
||||
function readPage(variant: PageVariant): ReadablePage | undefined {
|
||||
// `parse()` works on a copy of the document and reads styles and image sizes from the page,
|
||||
// so it is given the live document.
|
||||
const article = new Defuddle(document, { url: location.href }).parse();
|
||||
const wholePage = new Defuddle(document, { url: location.href, ...WHOLE_PAGE_OPTIONS }).parse();
|
||||
const options = variant === "wholePage" ? WHOLE_PAGE_OPTIONS : {};
|
||||
const result = new Defuddle(document, { url: location.href, ...options }).parse();
|
||||
|
||||
const page = {
|
||||
title: article.title,
|
||||
pageUrl: getPageLocationOrigin() + location.pathname + location.search,
|
||||
clipType: "page" as const,
|
||||
labels: pageLabels(article)
|
||||
};
|
||||
const whole = withContent(page, wholePage.content);
|
||||
if (!whole) {
|
||||
return undefined;
|
||||
}
|
||||
return { article: withContent(page, article.content), wholePage: whole };
|
||||
}
|
||||
|
||||
/** The clip of `html`, its images replaced by placeholders, or `undefined` when it shows nothing. */
|
||||
function withContent(page: Omit<ReadablePage, "content" | "images">, html: string): ReadablePage | undefined {
|
||||
// A document of its own loads none of the page's images and runs none of its code.
|
||||
const body = new DOMParser().parseFromString(html, "text/html").body;
|
||||
const body = new DOMParser().parseFromString(result.content, "text/html").body;
|
||||
if (!body.textContent.trim() && !body.querySelector("img")) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const images = getImages(body);
|
||||
return { ...page, content: body.innerHTML, images };
|
||||
return {
|
||||
title: result.title,
|
||||
content: body.innerHTML,
|
||||
images,
|
||||
pageUrl: getPageLocationOrigin() + location.pathname + location.search,
|
||||
clipType: "page",
|
||||
labels: pageLabels(result)
|
||||
};
|
||||
}
|
||||
|
||||
function pageLabels(article: { author: string, published: string }) {
|
||||
|
||||
@@ -44,20 +44,29 @@ describe("createLink", () => {
|
||||
describe("extractPage", () => {
|
||||
afterEach(() => {
|
||||
Reflect.deleteProperty(fakeBrowser, "scripting");
|
||||
Reflect.deleteProperty(globalThis, "triliumReadPage");
|
||||
});
|
||||
|
||||
it("runs the readable-page script in the tab and returns its result", async () => {
|
||||
it("injects the readable-page script, then reads the page as asked", async () => {
|
||||
const page = { title: "Article", content: "<p>Body</p>" };
|
||||
const executeScript = vi.fn()
|
||||
.mockResolvedValueOnce([ { result: page } ])
|
||||
.mockResolvedValueOnce([ { result: null } ])
|
||||
.mockResolvedValueOnce([]);
|
||||
const readPage = vi.fn((variant: string) => variant === "article" ? page : undefined);
|
||||
const executeScript = vi.fn(async ({ func, args }: { func?: (variant: string) => unknown, args?: [ string ] }) => {
|
||||
if (!func || !args) return [];
|
||||
Object.assign(globalThis, { triliumReadPage: readPage });
|
||||
return [ { result: func(...args) } ];
|
||||
});
|
||||
Object.assign(fakeBrowser, { scripting: { executeScript } });
|
||||
|
||||
expect(await extractPage(7)).toBe(page);
|
||||
expect(executeScript).toHaveBeenCalledWith({ target: { tabId: 7 }, files: [ "/readable-page.js" ] });
|
||||
expect(await extractPage(7)).toBeUndefined();
|
||||
expect(await extractPage(7)).toBeUndefined();
|
||||
expect(await extractPage(7, "article")).toBe(page);
|
||||
expect(executeScript.mock.calls.map(([ injection ]) => injection)).toEqual([
|
||||
{ target: { tabId: 7 }, files: [ "/readable-page.js" ] },
|
||||
{ target: { tabId: 7 }, func: expect.any(Function), args: [ "article" ] }
|
||||
]);
|
||||
expect(await extractPage(7, "wholePage")).toBeUndefined();
|
||||
expect(readPage.mock.calls).toEqual([ [ "article" ], [ "wholePage" ] ]);
|
||||
|
||||
executeScript.mockResolvedValue([]);
|
||||
expect(await extractPage(7, "article")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -13,22 +13,27 @@ export interface ReadablePage {
|
||||
labels: Record<string, string>;
|
||||
}
|
||||
|
||||
/** The two ways to clip a page: its article, when Defuddle finds one, and all of it. */
|
||||
export interface ExtractedPage {
|
||||
article?: ReadablePage;
|
||||
wholePage: ReadablePage;
|
||||
/** The two ways to read a page: its article, as Defuddle finds it, and all of it. */
|
||||
export type PageVariant = "article" | "wholePage";
|
||||
|
||||
declare global {
|
||||
/** Registered in the tab's isolated world by `entrypoints/readable-page/index.ts`. */
|
||||
var triliumReadPage: (variant: PageVariant) => ReadablePage | undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs `entrypoints/readable-page/index.ts` in the tab and returns the page it read, or `undefined`
|
||||
* when the page shows nothing. Throws where the extension cannot run scripts, such as browser pages.
|
||||
* Injects `entrypoints/readable-page/index.ts` into the tab and reads the page as `variant`, or
|
||||
* returns `undefined` when that reading shows nothing. Throws where the extension cannot run
|
||||
* scripts, such as browser pages.
|
||||
*/
|
||||
export async function extractPage(tabId: number): Promise<ExtractedPage | undefined> {
|
||||
export async function extractPage(tabId: number, variant: PageVariant): Promise<ReadablePage | undefined> {
|
||||
await browser.scripting.executeScript({ target: { tabId }, files: [ "/readable-page.js" ] });
|
||||
const [ injection ] = await browser.scripting.executeScript({
|
||||
target: { tabId },
|
||||
files: [ "/readable-page.js" ]
|
||||
func: (variant: PageVariant) => globalThis.triliumReadPage(variant),
|
||||
args: [ variant ]
|
||||
});
|
||||
return (injection?.result ?? undefined) as ExtractedPage | undefined;
|
||||
return (injection?.result ?? undefined) as ReadablePage | undefined;
|
||||
}
|
||||
|
||||
export function randomString(len: number) {
|
||||
|
||||
Reference in New Issue
Block a user