mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 10:02:41 +00:00
🩹 fix: PPTX CDN embeds slide-list fallback + detects empty renders
Manual e2e + DOM inspection on PR #12934: pptx-preview silently
produces empty `.pptx-preview-wrapper` placeholders for pptxgenjs-
generated decks. The library parses the file enough to create the
960×540 host element with a black bg, then fails to populate it.
The outer Promise resolves "successfully" — no throw, no rejection,
the bootstrap thinks rendering succeeded — and the user sees a black
rectangle with no content and no fallback message.
Fix mirrors the DOCX mammoth-fallback pattern from commit 0c0b0ce88:
1. **Server side**: `pptxToHtml` now renders the slide-list body
(`<ol class="lc-pptx-list">...`) via the new `renderPptxSlidesBody`
helper, then embeds it inside the CDN doc via the new
`buildPptxCdnDocument(base64, slideListFallbackBody)` signature.
Combined-doc size budget mirrors the DOCX pattern: if the CDN doc
would exceed `OFFICE_HTML_OUTPUT_CAP` (512 KB), drop to slide-list
only.
2. **Iframe bootstrap**: new `hasRenderedContent()` check after
`wrapSlides()` walks each `.lc-slide-wrap` looking for actual
child content inside pptx-preview's emitted slide nodes. If every
wrap is empty, fires `showFallback('renderer-produced-empty-
wrappers ...')` which reveals the embedded slide-list view
instead of the previous static "Preview unavailable" message.
3. **CSS**: slide-list rules extracted to `PPTX_SLIDE_LIST_CSS`
constant so they can be inlined into both the standalone slide-
list document AND the CDN doc's `<style>` block (CSP `style-src`
is `'unsafe-inline'` only — no external sheets).
`renderPptxSlidesHtml` now delegates to `renderPptxSlidesBody`
wrapped in `wrapAsDocument` — single source of truth for the slide
markup.
Tests (506 passing, +1 vs before): existing `pptxToHtmlViaCdn`
call sites updated for the new fallback-body argument; new
regression test pins `hasRenderedContent`, the
`renderer-produced-empty-wrappers` reason string, the embedded
fallback structure, and the inlined slide-list CSS.
This commit is contained in:
Danny Avila
parent
151dea768c
commit
414c5bf46f
2 changed files with 153 additions and 29 deletions
|
|
@ -419,14 +419,23 @@ describe('Office HTML producers', () => {
|
|||
expect(html).toContain('id="lc-doc-data"');
|
||||
expect(html).toContain('pptxPreview.init');
|
||||
expect(html).toContain('cdn.jsdelivr.net/npm/pptx-preview@');
|
||||
// Slide-list signature must NOT appear (those are <ol class="lc-pptx-list">).
|
||||
expect(html).not.toContain('class="lc-pptx-list"');
|
||||
/* The slide-list `<ol class="lc-pptx-list">` is now embedded
|
||||
* INSIDE the CDN doc as a runtime fallback for the silent-
|
||||
* empty-render case (pptx-preview's compatibility issues with
|
||||
* pptxgenjs decks). It lives in the hidden `#lc-fallback`
|
||||
* block; the bootstrap reveals it when `hasRenderedContent()`
|
||||
* detects empty wrappers. Manual e2e on PR #12934. */
|
||||
expect(html).toContain('class="lc-pptx-list"');
|
||||
expect(html).toContain('id="lc-fallback"');
|
||||
});
|
||||
|
||||
describe('CDN-rendered path', () => {
|
||||
test('embeds the binary as base64 that round-trips to the original bytes', async () => {
|
||||
const original = await buildPptx([{ title: 'Test' }]);
|
||||
const html = await _internal.pptxToHtmlViaCdn(original);
|
||||
const html = await _internal.pptxToHtmlViaCdn(
|
||||
original,
|
||||
'<ol class="lc-pptx-list"><li>fb</li></ol>',
|
||||
);
|
||||
const match = html.match(
|
||||
/<script id="lc-doc-data" type="application\/octet-stream;base64">([^<]*)<\/script>/,
|
||||
);
|
||||
|
|
@ -437,7 +446,10 @@ describe('Office HTML producers', () => {
|
|||
|
||||
test('pins pptx-preview to a specific version with SRI integrity', async () => {
|
||||
const pptx = await buildPptx([{ title: 'X' }]);
|
||||
const html = await _internal.pptxToHtmlViaCdn(pptx);
|
||||
const html = await _internal.pptxToHtmlViaCdn(
|
||||
pptx,
|
||||
'<ol class="lc-pptx-list"><li>fb</li></ol>',
|
||||
);
|
||||
expect(html).toContain('https://cdn.jsdelivr.net/npm/pptx-preview@1.0.7/');
|
||||
expect(html).toMatch(/integrity="sha384-[A-Za-z0-9+/=]+"/);
|
||||
expect(html).toContain('crossorigin="anonymous"');
|
||||
|
|
@ -445,7 +457,10 @@ describe('Office HTML producers', () => {
|
|||
|
||||
test('CSP locks the iframe down: connect-src restricted, no eval, no base/form tampering', async () => {
|
||||
const pptx = await buildPptx([{ title: 'X' }]);
|
||||
const html = await _internal.pptxToHtmlViaCdn(pptx);
|
||||
const html = await _internal.pptxToHtmlViaCdn(
|
||||
pptx,
|
||||
'<ol class="lc-pptx-list"><li>fb</li></ol>',
|
||||
);
|
||||
const cspMatch = html.match(
|
||||
/<meta http-equiv="Content-Security-Policy" content="([^"]+)">/,
|
||||
);
|
||||
|
|
@ -473,10 +488,19 @@ describe('Office HTML producers', () => {
|
|||
|
||||
test('exposes a fallback message that surfaces if the renderer fails to load or times out', async () => {
|
||||
const pptx = await buildPptx([{ title: 'X' }]);
|
||||
const html = await _internal.pptxToHtmlViaCdn(pptx);
|
||||
const html = await _internal.pptxToHtmlViaCdn(
|
||||
pptx,
|
||||
'<ol class="lc-pptx-list"><li>fb</li></ol>',
|
||||
);
|
||||
// Visible loading state + fallback that swaps in on error.
|
||||
expect(html).toContain('Loading preview…');
|
||||
expect(html).toContain('Preview unavailable');
|
||||
/* Fallback now embeds the slide-list view + a notice
|
||||
* explaining the high-fidelity renderer failed. The old
|
||||
* static "Preview unavailable" text is replaced with the
|
||||
* actual readable content. Manual e2e on PR #12934 (pptxgenjs
|
||||
* compatibility issue with pptx-preview). */
|
||||
expect(html).toContain('High-fidelity renderer unavailable');
|
||||
expect(html).toContain('class="lc-pptx-list"');
|
||||
// The renderer-not-loaded check.
|
||||
expect(html).toContain("typeof pptxPreview === 'undefined'");
|
||||
// The unhandledrejection + error listeners — pptx-preview's
|
||||
|
|
@ -504,6 +528,32 @@ describe('Office HTML producers', () => {
|
|||
expect(_internal.MAX_PPTX_CDN_BINARY_BYTES).toBe(350 * 1024);
|
||||
});
|
||||
|
||||
test('embeds the slide-list fallback in the CDN doc with empty-render detection', async () => {
|
||||
/* pptx-preview silently produces empty 960×540 wrappers for
|
||||
* pptxgenjs-generated decks (manual e2e on PR #12934 — it
|
||||
* parses the file enough to create the placeholder, then
|
||||
* fails to populate it). The bootstrap now detects the
|
||||
* empty-render case via `hasRenderedContent()` and reveals
|
||||
* the embedded slide-list fallback so the user always gets
|
||||
* readable content. */
|
||||
const pptx = await buildPptx([{ title: 'A' }, { title: 'B' }]);
|
||||
const html = await _internal.pptxToHtmlViaCdn(
|
||||
pptx,
|
||||
'<ol class="lc-pptx-list"><li>SENTINEL_FALLBACK</li></ol>',
|
||||
);
|
||||
/* Slide-list fallback body is embedded inside `#lc-fallback`. */
|
||||
expect(html).toContain('id="lc-fallback"');
|
||||
expect(html).toContain('SENTINEL_FALLBACK');
|
||||
/* Empty-render detection helper exists in the bootstrap. */
|
||||
expect(html).toContain('hasRenderedContent');
|
||||
expect(html).toContain('renderer-produced-empty-wrappers');
|
||||
/* The slide-list fallback CSS is inlined so the fallback
|
||||
* renders with the same look as the standalone slide-list
|
||||
* path (CSP locks `style-src` to inline only). */
|
||||
expect(html).toContain('.lc-pptx-list');
|
||||
expect(html).toContain('.lc-pptx-slide');
|
||||
});
|
||||
|
||||
test('bootstrap wraps + scales each slide so it fits the iframe width', async () => {
|
||||
/* pptx-preview emits slides at the init dimensions (960×540
|
||||
* by default). Without post-processing, narrow artifact panels
|
||||
|
|
@ -518,7 +568,10 @@ describe('Office HTML producers', () => {
|
|||
* — its internal pipeline holds references to the appended
|
||||
* slides and broke when we moved them under a parent wrap. */
|
||||
const pptx = await buildPptx([{ title: 'A' }, { title: 'B' }]);
|
||||
const html = await _internal.pptxToHtmlViaCdn(pptx);
|
||||
const html = await _internal.pptxToHtmlViaCdn(
|
||||
pptx,
|
||||
'<ol class="lc-pptx-list"><li>fb</li></ol>',
|
||||
);
|
||||
/* The wrapper class used by the CSS rules. */
|
||||
expect(html).toContain('lc-slide-wrap');
|
||||
/* The wrap function + the per-slide scale function. The
|
||||
|
|
|
|||
|
|
@ -944,14 +944,11 @@ function decodeXmlEntities(input: string): string {
|
|||
return input.replace(/&(?:lt|gt|amp|quot|apos);/g, (m) => XML_ENTITIES[m] ?? m);
|
||||
}
|
||||
|
||||
function renderPptxSlidesHtml(slides: PptxSlide[]): string {
|
||||
if (slides.length === 0) {
|
||||
return wrapAsDocument(
|
||||
'<p class="lc-banner">This presentation contains no readable slides.</p>',
|
||||
);
|
||||
}
|
||||
|
||||
const slideCss = `
|
||||
/* CSS for the slide-list view. Extracted to a constant so it can also
|
||||
* be inlined into the CDN doc when the slide-list is embedded as a
|
||||
* fallback there — Codex feedback / manual e2e on PR #12934 ("pptx-
|
||||
* preview created an empty wrapper for the pptxgenjs deck"). */
|
||||
const PPTX_SLIDE_LIST_CSS = `
|
||||
.lc-pptx-list { display: flex; flex-direction: column; gap: 16px; padding: 0; margin: 0; list-style: none; }
|
||||
.lc-pptx-slide {
|
||||
border: 1px solid var(--border);
|
||||
|
|
@ -974,6 +971,17 @@ function renderPptxSlidesHtml(slides: PptxSlide[]): string {
|
|||
.lc-pptx-slide-empty { color: var(--muted); font-style: italic; margin: 0; }
|
||||
`.trim();
|
||||
|
||||
/**
|
||||
* Build the slide-list body HTML (just the `<ol>` element, no document
|
||||
* wrapper). Used both for standalone slide-list rendering and for
|
||||
* embedding inside the CDN doc as an air-gap / parser-failure
|
||||
* fallback — same pattern as the mammoth fallback inside the DOCX CDN
|
||||
* doc. Returns an empty-state banner for zero-slide decks.
|
||||
*/
|
||||
function renderPptxSlidesBody(slides: PptxSlide[]): string {
|
||||
if (slides.length === 0) {
|
||||
return '<p class="lc-banner">This presentation contains no readable slides.</p>';
|
||||
}
|
||||
const items = slides
|
||||
.map((slide) => {
|
||||
const titleHtml = slide.title
|
||||
|
|
@ -992,8 +1000,11 @@ function renderPptxSlidesHtml(slides: PptxSlide[]): string {
|
|||
</li>`;
|
||||
})
|
||||
.join('\n');
|
||||
return `<ol class="lc-pptx-list">${items}</ol>`;
|
||||
}
|
||||
|
||||
return wrapAsDocument(`<ol class="lc-pptx-list">${items}</ol>`, slideCss);
|
||||
function renderPptxSlidesHtml(slides: PptxSlide[]): string {
|
||||
return wrapAsDocument(renderPptxSlidesBody(slides), PPTX_SLIDE_LIST_CSS);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1069,7 +1080,7 @@ const MAX_PPTX_CDN_BINARY_BYTES = 350 * 1024;
|
|||
* iframe via `transform: scale(...)`. The slides scroll vertically
|
||||
* once the renderer paints them.
|
||||
*/
|
||||
function buildPptxCdnDocument(base64: string): string {
|
||||
function buildPptxCdnDocument(base64: string, slideListFallbackBody: string): string {
|
||||
/* PPTX-specific CSP relaxations vs DOCX:
|
||||
* - `worker-src blob:` — pptx-preview's bundled echarts dep spins up
|
||||
* Web Workers via blob: URLs for chart rendering. Without this,
|
||||
|
|
@ -1137,16 +1148,25 @@ html, body { margin: 0; padding: 0; background: var(--bg); color: var(--fg); fon
|
|||
left: 0;
|
||||
transform-origin: top left;
|
||||
}
|
||||
#lc-fallback { padding: 24px; font-size: 14px; line-height: 1.5; color: var(--muted); text-align: center; }
|
||||
#lc-fallback { padding: 16px; font-size: 14px; line-height: 1.5; color: var(--fg); }
|
||||
#lc-fallback-notice { font-size: 12px; color: var(--muted); border-bottom: 1px solid var(--border); padding-bottom: 8px; margin: 0 0 16px; }
|
||||
.lc-pptx-loading { display: flex; align-items: center; justify-content: center; height: 60vh; color: var(--muted); font-size: 14px; }
|
||||
/* Slide-list fallback styles. Inlined here so the fallback renders
|
||||
* with the same look as the standalone slide-list path when the CDN
|
||||
* renderer fails. CSS variable defaults match wrapAsDocument so
|
||||
* dark-mode borders/muted text resolve sensibly. */
|
||||
:root { --border: rgba(128, 128, 128, 0.25); --link: #2563eb; }
|
||||
@media (prefers-color-scheme: dark) { :root { --border: rgba(128, 128, 128, 0.35); --link: #60a5fa; } }
|
||||
${PPTX_SLIDE_LIST_CSS}
|
||||
</style>
|
||||
<script src="${PPTX_PREVIEW_CDN.pptxPreview.src}" integrity="${PPTX_PREVIEW_CDN.pptxPreview.integrity}" crossorigin="anonymous"></script>
|
||||
</head>
|
||||
<body>
|
||||
<div id="lc-render"><div class="lc-pptx-loading">Loading preview…</div></div>
|
||||
<div id="lc-fallback" hidden>
|
||||
<p style="margin: 0 0 8px;">Preview unavailable. Please download the file to view it.</p>
|
||||
<details style="font-size: 12px; color: var(--muted); margin: 0;">
|
||||
<p id="lc-fallback-notice">High-fidelity renderer unavailable or returned no slide content. Showing the simplified slide-list view below.</p>
|
||||
${slideListFallbackBody}
|
||||
<details style="font-size: 12px; color: var(--muted); margin: 16px 0 0;">
|
||||
<summary style="cursor: pointer;">Diagnostic details</summary>
|
||||
<div id="lc-fallback-reason" style="margin-top: 6px; font-family: ui-monospace, SFMono-Regular, Menlo, monospace; word-break: break-word;"></div>
|
||||
</details>
|
||||
|
|
@ -1162,8 +1182,10 @@ html, body { margin: 0; padding: 0; background: var(--bg); color: var(--fg); fon
|
|||
if (loading) { loading.remove(); }
|
||||
var container = document.getElementById('lc-render');
|
||||
if (container) {
|
||||
// Wipe any partial render the library left behind so the
|
||||
// fallback message isn't competing with a half-rendered slide.
|
||||
/* Hide the render slot (which may contain pptx-previews
|
||||
* empty/black wrappers) so the slide-list fallback owns the
|
||||
* viewport. */
|
||||
container.hidden = true;
|
||||
container.innerHTML = '';
|
||||
}
|
||||
var fallback = document.getElementById('lc-fallback');
|
||||
|
|
@ -1300,8 +1322,32 @@ html, body { margin: 0; padding: 0; background: var(--bg); color: var(--fg); fon
|
|||
}
|
||||
}
|
||||
|
||||
/* Detect the silent-failure case: pptx-preview created the slide
|
||||
* wrappers but didnt populate them with content. Common with
|
||||
* pptxgenjs-generated PPTXs — pptx-previews parser handles
|
||||
* Microsoft PowerPoint XML but chokes on subtle convention
|
||||
* differences and produces empty 960x540 wrappers. Check if every
|
||||
* wrapped slide has zero children; if so, the render failed and
|
||||
* the slide-list fallback should take over. Manual e2e on PR
|
||||
* #12934. */
|
||||
function hasRenderedContent() {
|
||||
var wraps = container.querySelectorAll('.lc-slide-wrap');
|
||||
if (wraps.length === 0) { return false; }
|
||||
for (var i = 0; i < wraps.length; i++) {
|
||||
var inner = wraps[i].firstElementChild;
|
||||
if (inner && inner.children.length > 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function finalize() {
|
||||
wrapSlides();
|
||||
if (!hasRenderedContent()) {
|
||||
showFallback('renderer-produced-empty-wrappers (pptx-preview compatibility issue with this PPTX format — falling back to slide-list view)');
|
||||
return;
|
||||
}
|
||||
container.style.visibility = 'visible';
|
||||
/* Resize observer set up AFTER the wrap so it doesnt fire
|
||||
* during the wrap pass. */
|
||||
|
|
@ -1337,8 +1383,21 @@ html, body { margin: 0; padding: 0; background: var(--bg); color: var(--fg); fon
|
|||
</html>`;
|
||||
}
|
||||
|
||||
async function pptxToHtmlViaCdn(buffer: Buffer): Promise<string> {
|
||||
return buildPptxCdnDocument(buffer.toString('base64'));
|
||||
/**
|
||||
* Render the slide-list body (just the `<ol>` HTML) for a buffer.
|
||||
* Used by `pptxToHtml` to embed as a fallback inside the CDN doc and
|
||||
* by `pptxToSlideListHtmlInternal` for the standalone path. */
|
||||
async function renderPptxSlidesBodyForBuffer(buffer: Buffer): Promise<string> {
|
||||
const rawSlides = await extractPptxSlideXml(buffer);
|
||||
const slides: PptxSlide[] = rawSlides.map(({ number, xml }) => {
|
||||
const { title, body } = extractSlideText(xml);
|
||||
return { number, title, body };
|
||||
});
|
||||
return renderPptxSlidesBody(slides);
|
||||
}
|
||||
|
||||
async function pptxToHtmlViaCdn(buffer: Buffer, slideListFallbackBody: string): Promise<string> {
|
||||
return buildPptxCdnDocument(buffer.toString('base64'), slideListFallbackBody);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1370,13 +1429,25 @@ export async function pptxToHtml(buffer: Buffer): Promise<string> {
|
|||
if (lo) {
|
||||
return lo;
|
||||
}
|
||||
if (isOfficePreviewCdnDisabled()) {
|
||||
if (isOfficePreviewCdnDisabled() || buffer.length > MAX_PPTX_CDN_BINARY_BYTES) {
|
||||
return pptxToSlideListHtmlInternal(buffer);
|
||||
}
|
||||
if (buffer.length <= MAX_PPTX_CDN_BINARY_BYTES) {
|
||||
return pptxToHtmlViaCdn(buffer);
|
||||
/* Render the slide-list first so we can embed it inside the CDN doc
|
||||
* as a runtime fallback. pptx-preview is unreliable on PPTXs not
|
||||
* generated by Microsoft PowerPoint (e.g. pptxgenjs decks) — it
|
||||
* silently produces empty wrappers. The iframe bootstrap detects
|
||||
* the empty-render case and reveals this slide-list fallback so the
|
||||
* user always gets readable content. Manual e2e on PR #12934. */
|
||||
const slideListBody = await renderPptxSlidesBodyForBuffer(buffer);
|
||||
const cdnDoc = await pptxToHtmlViaCdn(buffer, slideListBody);
|
||||
/* Combined size budget: if base64 binary + slide-list fallback +
|
||||
* wrapper would exceed the cache cap, drop CDN entirely and ship
|
||||
* the slide-list standalone. Same pattern as the DOCX dispatcher's
|
||||
* size budget. */
|
||||
if (Buffer.byteLength(cdnDoc, 'utf-8') > OFFICE_HTML_OUTPUT_CAP) {
|
||||
return pptxToSlideListHtmlInternal(buffer);
|
||||
}
|
||||
return pptxToSlideListHtmlInternal(buffer);
|
||||
return cdnDoc;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue