143 lines
9.0 KiB
Markdown
143 lines
9.0 KiB
Markdown
# File viewer follow-ups (`@wrnexus/viewer`)
|
|
|
|
Date: 2026-09-09
|
|
Status: implemented — accepted limits retained
|
|
Follows: [file viewer design](2026-09-08-file-viewer-design.md), whose build order is complete
|
|
|
|
Build order steps 1-5 are implemented and the package renders every group in the format
|
|
matrix. What remains falls into four kinds: defects found after the fact, work the design
|
|
described but that was never built, one verification gap, and limits that were accepted
|
|
deliberately and should stay accepted until something forces the issue.
|
|
|
|
Nothing here blocks using the package. The first section is the only part with a
|
|
correctness argument behind it.
|
|
|
|
## Implementation result (2026-09-09)
|
|
|
|
- Stable conversion identities and authorization before cache hits; source streams released on hits.
|
|
- Exported `firstOf` / `sameOriginSource`; demo uses the composite with signed uploads.
|
|
- Renderer and chrome `.wrn` presentation components integrated through preserved DOM hooks.
|
|
- Sanitized HTML preview by default, with Source toggle and strict sandbox/CSP.
|
|
- Multi-file filmstrip consuming thumbnails, plus on-demand audio waveform and seek slider.
|
|
- Extracted-text UI and bounded public helper consuming the text route.
|
|
- Explicit host base URI for conversion/optional-module helpers, with regression tests.
|
|
- Suppressed plugin factories are skipped before import/invocation.
|
|
- Same-origin framing is now the framework default, explicitly approved by the user;
|
|
explicit DENY still sets restrictive framing headers.
|
|
- pdf.js success observed in ordinary Chrome: a completed page canvas visibly painted the
|
|
sample document text. Waveform generation and filmstrip switching also verified there.
|
|
|
|
The per-process cache, streaming-driver byte-range limitation and host-binary DOCX fidelity
|
|
remain accepted as this plan specifies. No distributed-cache or uploader-driver redesign was
|
|
introduced. The findings below are the original rationale, retained for context.
|
|
|
|
## Defects worth fixing first
|
|
|
|
**A rotating token defeats the conversion cache.** `packages/viewer/src/convert/service.ts`
|
|
keys cached output on `src`, and for a private uploader object `src` is
|
|
`/api/viewer/file?token=…` — a link that rotates every `ttlMs` (5 minutes by default). Every
|
|
view after the link is re-minted is a cache miss, so exactly the files most expensive to
|
|
convert (a 40 MB `.docx` through LibreOffice) are the ones that never hit. The cache key must
|
|
identify the _object_, not the URL that happens to reach it: have `resolveSource` return an
|
|
optional stable identity (`store:key` for uploader sources, the URL for everything else) and
|
|
hash that instead. Until then, private conversions re-run on a timer.
|
|
|
|
**The example app can no longer convert its own static files.**
|
|
`examples/basic-app/wrnexus.config.ts` sets `resolveSource: uploadedConversionSource()`, which
|
|
replaces the same-origin default outright. Uploader objects convert; the page's own
|
|
`/viewer/sample.docx` cannot, because it has no signed link. On this machine that is invisible
|
|
— no LibreOffice, so the request stops at 415 — and it would surface only on a host with the
|
|
binaries installed. The example should use whichever composite lands below.
|
|
|
|
**There is no way to accept more than one kind of source.** `ConversionOptions.resolveSource`
|
|
is a single function, so an app that stores some files in an uploader store and serves others
|
|
statically has to hand-write the fan-out. Ship a composer — `firstOf(uploadedConversionSource(),
|
|
sameOriginSource())` — and export `sameOriginSource` so the default is nameable rather than
|
|
private to the module. Small surface, removes the trap the example fell into.
|
|
|
|
## Unbuilt from the design
|
|
|
|
**Renderer and chrome `.wrn` components.** The design's module layout lists `ViewerShell`,
|
|
`ViewerToolbar`, `ImageViewer`, `MediaViewer`, `PdfViewer`, `CodeViewer`, `SheetViewer`,
|
|
`DocViewer`, `ArchiveViewer` and `DownloadCard`; what exists is one `Viewer.wrn` plus
|
|
imperative browser adapters sharing a `ViewerMountContext`. The adapters work and the seam is
|
|
clean, so this is a presentation question, not a correctness one: components would let an app
|
|
restyle the chrome through the normal component-override path instead of registering a
|
|
renderer. Worth doing only if an app actually asks to restyle it.
|
|
|
|
**HTML still renders as inert source.** The design's security section describes `.html` in the
|
|
same sandboxed, CSP-restricted frame as SVG. The frame exists (`renderers/frame.ts`) and SVG
|
|
and `.docx` both use it; pointing `.html` at it is a small change. It was left out because
|
|
inert source is strictly safer and nobody had asked for a rendered preview. Doing it means
|
|
deciding what the default view is — preview with a source toggle, or the reverse.
|
|
|
|
**`/api/viewer/thumb` and `/api/viewer/text` are mounted but unused.** The routes work and the
|
|
converter implements both. Nothing calls them because the two features that would — a
|
|
multi-file filmstrip, and feeding extracted text to search — are not built. Either build a
|
|
consumer or drop the routes; a live endpoint with no caller is a maintenance cost and an
|
|
attack surface for nothing.
|
|
|
|
**Audio waveform and multi-file filmstrip.** Both are in the design, neither is built. The
|
|
filmstrip is the one with a dependency: it wants `thumb`, and it wants a story for what a
|
|
viewer showing _several_ files does to the toolbar and the disposal path.
|
|
|
|
## The verification gap
|
|
|
|
**pdf.js rasterisation has never been observed working.** The library and worker are served
|
|
verbatim, `getDocument()` resolves and reports the page count in a real browser, and every
|
|
step up to `page.render()` was verified. `page.render()` itself never completes in this
|
|
environment, because pdf.js drives rasterisation from `requestAnimationFrame` and the
|
|
preview pane reports `document.hidden === true` permanently, even when fronted. The renderer
|
|
now bounds each page with a timeout and degrades to the native frame rather than hanging —
|
|
that degradation _is_ verified — but the success path is unproven. It needs one run in an
|
|
ordinary visible browser: open `/file-viewer` after `registerRenderer(richPdfRenderer())` and
|
|
confirm a canvas paints. Until someone does that, treat rich PDF as untested.
|
|
|
|
## Outside the package
|
|
|
|
**Plugin discovery calls a suppressed manifest factory anyway.**
|
|
`packages/plugin/src/discovery.ts:353` decides whether to keep a manifest-built plugin _after_
|
|
constructing it, so a plugin configured explicitly in `wrnexus.config.ts` still has its
|
|
factory invoked a second time with no options. That silently cleared the viewer's conversion
|
|
config until `viewerPlugin` was changed to only ever arm, never disarm. Any plugin using the
|
|
config-global idiom will hit this. Already queued as its own task.
|
|
|
|
**`frame-ancestors 'none'` stops an app framing its own assets.**
|
|
`packages/core/src/headers.ts:119`, with `X-Frame-Options: DENY` alongside it. Embedding a
|
|
same-origin PDF — or anything else in a frame — requires every app to opt out, as
|
|
`examples/basic-app` now does. `'self'` would still block cross-origin clickjacking while
|
|
letting an app embed its own content. This is a framework-wide security default, so it is a
|
|
decision to take deliberately rather than a bug to fix in passing.
|
|
|
|
## Known limits, deliberately accepted
|
|
|
|
**The conversion cache is per-process.** A bounded `Map` with a TTL, so a multi-process deploy
|
|
converts once per process and a restart loses everything. Moving it to `@wrnexus/cache` with
|
|
the distributed invalidation layer is the obvious upgrade, and is only worth it once the key
|
|
is fixed — a shared cache keyed on a rotating token would be shared misses.
|
|
|
|
**Byte ranges are served only for buffered objects.** `uploads/service.ts:135` slices a
|
|
`Uint8Array` body for a `Range` request; a driver returning a stream answers the whole body
|
|
instead. Seeking a large private video therefore re-reads it. Fixing this properly means a
|
|
range-aware read on the driver contract, which belongs to `@wrnexus/uploader`, not here.
|
|
|
|
**Legacy `.doc` and `.ppt` remain converter-only.** The OOXML and OpenDocument families are
|
|
ZIP containers of XML and are now read in-package, and RTF is read with no dependency at all.
|
|
The pre-2007 OLE compound formats are a different problem — a binary record stream with no
|
|
public schema — and reading them in a browser is not worth the size or the risk. LibreOffice
|
|
handles them, and without it they keep the download card.
|
|
|
|
**`.docx` fidelity depends on a host binary.** Mammoth produces semantic HTML, not a page
|
|
layout, so the converter's PDF is preferred and mammoth is the fallback. An app with no
|
|
LibreOffice gets an approximation, and that is the honest ceiling of a pure-JS path.
|
|
|
|
**Three client helpers still read the global `document`.** `optional/load.ts:7` and
|
|
`convert/client.ts:13,36`. `resolveKey` was changed to take the host's `baseURI` when it turned
|
|
out to be untestable without a global; the others have the same latent problem and no test
|
|
covering them. Consistency work, not a live bug.
|
|
|
|
## Out of scope, still
|
|
|
|
Unchanged from the original design: editing or annotation, file management, OCR,
|
|
collaborative features, and executing anything from an uploaded file.
|