Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Security boundaries:
- Added and removed HTML must use one full-width pane. Never render an empty opposite pane.
- Edited HTML must render both revisions without visible **Before**, **After**, or **Ready** labels.
- Successful previews must not show resource panels or persistent diagnostics. Resource failures must remain actionable.
- Network failures and HTTP `408`, `425`, `429`, and `5xx` responses retry automatically three times with bounded exponential backoff. After exhaustion—or immediately for permanent failures—a failure-only **Retry** action clears cached metadata and refetches without reloading GitHub.
- Network failures and HTTP `408`, `425`, `429`, and `5xx` responses retry automatically three times with bounded exponential backoff. After exhaustion—or immediately for permanent failures—a failure-only **Retry** action refetches the selected HTML and its supported repository resource graph with fresh HTTP requests, without reloading GitHub or clearing unrelated caches. PR Retry keeps the same base/head commits and leaves other file previews alone. External resources and arbitrary runtime requests keep their normal browser behavior.
- Review comments and GitHub collapse/expand behavior must continue working in Preview.
- GitHub native and extension fallback cards must never produce duplicate previews.
- Long HTML must remain scrollable. Edited panes must synchronize semantically when stable anchors exist and proportionally otherwise.
Expand Down
149 changes: 149 additions & 0 deletions e2e/extension.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2096,3 +2096,152 @@ function delay(milliseconds: number): Promise<void> {
setTimeout(resolve, milliseconds);
return promise;
}


test('Retry freshens only its blob source and transitive graph', async () => {
const { context, page, extensionId } = await launchWithExtension();
const unrelated = await context.newPage();
const selectedUrl = 'https://github.com/acme/reports/blob/main/selected/index.html';
const otherUrl = 'https://github.com/acme/reports/blob/main/other/index.html';
const requests: string[] = [];
let retrying = false;
const html = (version: string) => '<h1 id="version">' + version + '</h1>' +
'<style>@import "./theme.css";</style><img id="chart" src="./chart.png">' +
'<img src="./recover.png"><output id="module">waiting</output>' +
'<output id="modern-loaded"></output><output id="legacy-loaded"></output>' +
'<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="64" height="32">' +
'<defs><rect id="local-box" width="12" height="12"/></defs><use id="local-use" href="#local-box"/>' +
'<image id="modern-image" href="./diagram.svg" width="24" height="24" onload="document.querySelector(\'#modern-loaded\').textContent=\'loaded\'"/>' +
'<image id="legacy-image" xlink:href="./diagram.svg" x="32" width="24" height="24" onload="document.querySelector(\'#legacy-loaded\').textContent=\'loaded\'"/></svg>' +
'<script type="module" src="./main.js"></script>';
try {
// Routing disables Chromium's HTTP cache. The unit regression proves
// cache-mode behavior; this checks fresh graph rendering and request scope.
await context.route('**/*', async route => {
const url = route.request().url();
if (url === selectedUrl || url === otherUrl) {
const path = url === selectedUrl ? 'selected/index.html' : 'other/index.html';
await route.fulfill({ status: 200, contentType: 'text/html', body: githubBlobFixture(html('cached'), commit, path) });
return;
}
if (url.startsWith(rawBase + '/')) {
const path = url.slice(rawBase.length + 1);
requests.push(path);
const reload = retrying && path.startsWith('selected/');
const name = path.split('/').pop();
const version = reload ? 'fresh' : 'cached';
if (path === 'selected/recover.png' && !reload) {
await route.fulfill({ status: 400, body: 'resource unavailable' }); return;
}
const body = name === 'index.html' ? html(version) :
name === 'diagram.svg' ? '<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24"><rect width="24" height="24" fill="' + (reload ? 'lime' : 'red') + '"/></svg>' :
name === 'theme.css' ? '@import "./nested.css"; @font-face { font-family: report; src: url("./font.woff2") }' :
name === 'nested.css' ? 'h1 { color: ' + (reload ? 'rgb(4, 5, 6)' : 'rgb(1, 2, 3)') + '; background: url("./background.png") }' :
name === 'main.js' ? 'import { value } from "./child.js"; import("./lazy.js").then(() => document.querySelector("#module").textContent = value);' :
name === 'child.js' || name === 'lazy.js' ? 'export const value = "' + version + '";' : version;
await route.fulfill({ status: 200, contentType: name?.endsWith('.css') ? 'text/css' : name?.endsWith('.js') ? 'application/javascript' : name?.endsWith('.html') ? 'text/html' : 'application/octet-stream', headers: { 'cache-control': 'public, max-age=3600' }, body });
return;
}
if (url.startsWith('chrome-extension:')) { await route.continue(); return; }
await route.abort();
});
await page.goto(selectedUrl);
const selected = page.locator('.gh-html-preview-container');
await selected.getByText('View resource issues', { exact: true }).click();
await expect(selected.getByRole('button', { name: 'Retry' })).toBeVisible();
const initialFrame = selected.locator('iframe[title="Executable HTML preview"]').contentFrame();
await expect(initialFrame.locator('#modern-loaded')).toHaveText('loaded');
await expect(initialFrame.locator('#legacy-loaded')).toHaveText('loaded');
const cachedModernPixels = await initialFrame.locator('#modern-image').screenshot();
const cachedLegacyPixels = await initialFrame.locator('#legacy-image').screenshot();
await unrelated.goto(otherUrl);
const other = unrelated.locator('.gh-html-preview-container');
await expect(other.locator('iframe[title="Executable HTML preview"]').contentFrame().locator('#module')).toHaveText('cached');
// Observe RequestInit in the extension isolated world without changing it.
// Interception occurs before Chromium adds HTTP cache directives.
const cdp = await context.newCDPSession(page);
const worlds: number[] = [];
cdp.on('Runtime.executionContextCreated', event => worlds.push(event.context.id));
await cdp.send('Runtime.enable');
let extensionWorld: number | undefined;
for (const contextId of worlds) {
const identity = await cdp.send('Runtime.evaluate', { contextId,
expression: 'globalThis.chrome?.runtime?.id', returnByValue: true });
if (identity.result.value === extensionId) extensionWorld = contextId;
}
expect(extensionWorld).toBeDefined();
await cdp.send('Runtime.evaluate', { contextId: extensionWorld, expression: 'globalThis.retryRequests = []; const originalFetch = globalThis.fetch; globalThis.fetch = function(input, init) { retryRequests.push({ url: String(input), cache: init?.cache }); return originalFetch.call(this, input, init); }' });
const start = requests.length;
retrying = true;
await selected.getByRole('button', { name: 'Retry' }).click();
const frame = selected.locator('iframe[title="Executable HTML preview"]').contentFrame();
await expect(frame.locator('#version')).toHaveText('fresh');
await expect(frame.locator('#module')).toHaveText('fresh');
await expect(frame.locator('h1')).toHaveCSS('color', 'rgb(4, 5, 6)');
await expect(frame.locator('#chart')).toHaveAttribute('src', 'data:image/png;base64,' + Buffer.from('fresh').toString('base64'));
await expect(frame.locator('#modern-loaded')).toHaveText('loaded');
await expect(frame.locator('#legacy-loaded')).toHaveText('loaded');
await expect(frame.locator('#modern-image')).toHaveAttribute('href', /^data:image\/svg\+xml;base64,/);
const svgState = await frame.locator('svg').evaluate(element => ({
modern: element.querySelector('#modern-image')!.getAttribute('href'),
legacy: element.querySelector('#legacy-image')!.getAttributeNS('http://www.w3.org/1999/xlink', 'href'),
localWidth: (element.querySelector('#local-use') as SVGGraphicsElement).getBBox().width,
}));
expect(svgState.legacy).toBe(svgState.modern);
expect(Buffer.from(svgState.modern!.split(',')[1], 'base64').toString()).toContain('fill="lime"');
expect(svgState.localWidth).toBe(12);
// Same element bounds, red before Retry and lime after: prove the fresh
// SVG bytes were painted, not merely loaded or embedded in the document.
expect(await frame.locator('#modern-image').screenshot()).not.toEqual(cachedModernPixels);
expect(await frame.locator('#legacy-image').screenshot()).not.toEqual(cachedLegacyPixels);
await expect(selected.getByRole('button', { name: 'Retry' })).toHaveCount(0);
const observed = await cdp.send('Runtime.evaluate', { contextId: extensionWorld,
expression: 'retryRequests', returnByValue: true });
expect(observed.result.value).toHaveLength(11);
expect(observed.result.value.every((request: { url: string; cache: string }) =>
request.cache === 'reload' && request.url.startsWith(rawBase + '/selected/'))).toBe(true);
expect(requests.slice(start).every(path => path.startsWith('selected/'))).toBe(true);
expect(requests.slice(start).sort()).toEqual([
'index.html', 'theme.css', 'nested.css', 'font.woff2', 'background.png', 'chart.png', 'recover.png', 'diagram.svg', 'main.js', 'child.js', 'lazy.js',
].map(name => 'selected/' + name).sort());
await expect(other.locator('iframe[title="Executable HTML preview"]').contentFrame().locator('#version')).toHaveText('cached');
const normalStart = requests.length;
await unrelated.reload();
await expect(other.locator('iframe[title="Executable HTML preview"]').contentFrame().locator('#module')).toHaveText('cached');
expect(requests.slice(normalStart).every(path => path.startsWith('other/'))).toBe(true);
} finally {
await context.close();
}
});


test('Retry diagnoses repository external SVG use instead of emitting non-rendering data URLs', async () => {
const { context, page } = await launchWithExtension();
const sprite = '<svg xmlns="http://www.w3.org/2000/svg"><rect id="box" width="24" height="24"/></svg>';
const dataUse = 'data:image/svg+xml;base64,' + Buffer.from(sprite).toString('base64') + '#box';
const html = '<img src="./missing.png"><svg xmlns:xlink="http://www.w3.org/1999/xlink">' +
'<defs><rect id="box" width="24" height="24"/></defs>' +
'<use id="local" href="#box"/><use id="legacy-local" xlink:href="#box"/>' +
'<use id="repo" href="./symbols.svg#box"/>' +
'<use id="repo-legacy" xlink:href="./symbols.svg#box"/>' +
'<use id="data-use" href="' + dataUse + '"/>' +
'<use id="external" href="https://external.example/symbols.svg#box"/></svg>';
try {
const requested = await routeProductFixtures(context, html);
await page.goto(blobUrl);
const container = page.locator('.gh-html-preview-container');
await container.getByText('View resource issues', { exact: true }).click();
await container.getByRole('button', { name: 'Retry' }).click();
await expect(container.getByText(/Embed the SVG symbols in this HTML/)).toHaveCount(2);
const frame = container.locator('iframe[title="Executable HTML preview"]').contentFrame();
await expect(frame.locator('#repo')).not.toHaveAttribute('href');
await expect(frame.locator('#repo-legacy')).not.toHaveAttribute('xlink:href');
await expect(frame.locator('#data-use')).toHaveAttribute('href', dataUse);
await expect(frame.locator('#external')).toHaveAttribute('href', 'https://external.example/symbols.svg#box');
const widths = await frame.locator('svg').evaluate(svg => Array.from(svg.querySelectorAll('use')).map(use => [use.id, use.getBBox().width]));
expect(Object.fromEntries(widths)).toEqual({ local: 24, 'legacy-local': 24, repo: 0, 'repo-legacy': 0, 'data-use': 0, external: 0 });
expect(requested).not.toContain(rawBase + '/reports/weekly/symbols.svg');
} finally {
await context.close();
}
});
8 changes: 5 additions & 3 deletions entrypoints/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ export default defineContentScript({
},
});

async function ensureResolved(route: RouteState): Promise<void> {
async function ensureResolved(route: RouteState, refresh = false): Promise<void> {
if (!route.active || route.controller.signal.aborted) return;
if (route.render || route.resolving) return route.resolving ?? Promise.resolve();
const controller = route.controller;
Expand All @@ -371,17 +371,19 @@ async function ensureResolved(route: RouteState): Promise<void> {
privateRepo: route.isPrivate,
});
const sourceHtml =
route.sourceHtml ??
(!refresh ? route.sourceHtml : null) ??
(
await fetchRepositoryFile(route.repoRef, controller.signal, {
privateRepo: route.isPrivate,
refresh,
})
).text;
controller.signal.throwIfAborted();
const result = await resolveHtml(sourceHtml, {
target: 'sandbox-private',
repoRef: route.repoRef,
privateRepo: route.isPrivate,
refresh,
signal: controller.signal,
});
controller.signal.throwIfAborted();
Expand Down Expand Up @@ -516,7 +518,7 @@ function retryResolution(route: RouteState): void {
route.details.replaceChildren();
route.details.style.display = 'none';
route.previewArea.replaceChildren(message('Retrying preview resources…'));
void ensureResolved(route);
void ensureResolved(route, true);
}

function createActionButton(
Expand Down
12 changes: 8 additions & 4 deletions entrypoints/pr.content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1422,10 +1422,8 @@ async function renderRichComparison(
state.headRender?.destroy();
state.baseRender = null;
state.headRender = null;
state.comparison = null;
routeState.metadata = null;
routeState.fileMetadata = null;
routeState.sessionPullData = null;
// Keep this card pinned to its comparison commits and leave shared route
// metadata/in-flight work alone. Failed metadata promises clear themselves.
}
state.retryableFailures = 0;
const controller = new AbortController();
Expand Down Expand Up @@ -1474,6 +1472,7 @@ async function renderRichComparison(
routeState,
controller.signal,
info,
force,
),
);
}
Expand All @@ -1487,6 +1486,7 @@ async function renderRichComparison(
routeState,
controller.signal,
info,
force,
),
);
}
Expand Down Expand Up @@ -1539,6 +1539,7 @@ async function renderComparisonSide(
routeState: DiffRouteState,
signal: AbortSignal,
knownInfo: DiffFileInfo | null,
refresh: boolean,
): Promise<boolean> {
const area = sideName === 'base' ? state.baseArea : state.headArea;
try {
Expand All @@ -1551,6 +1552,7 @@ async function renderComparisonSide(
try {
file = await fetchRepositoryFile(repoRef, signal, {
privateRepo: side.privateRepo,
refresh,
});
} catch (initialError) {
const info = await getDiffFileInfo(
Expand All @@ -1567,6 +1569,7 @@ async function renderComparisonSide(
repoRef = sideRepoRef(side, path);
file = await fetchRepositoryFile(repoRef, signal, {
privateRepo: side.privateRepo,
refresh,
});
} else if (sideName === 'base' && info?.status === 'added') {
throw new ExpectedMissingSideError(
Expand All @@ -1585,6 +1588,7 @@ async function renderComparisonSide(
target: 'sandbox-private',
repoRef,
privateRepo,
refresh,
signal,
});
signal.throwIfAborted();
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "github-html-preview",
"description": "Safely preview public HTML files and pull-request changes on GitHub",
"private": true,
"version": "0.6.6",
"version": "0.6.7",
"type": "module",
"scripts": {
"dev": "wxt",
Expand Down
17 changes: 16 additions & 1 deletion tests/blobViewChoice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe('blob view choice', () => {
await navigate('second.html', false);
expect(mocks.fetch).toHaveBeenCalledWith(
{ owner: 'acme', repo: 'reports', ref: 'main', path: 'second.html' },
expect.any(AbortSignal), { privateRepo: false },
expect.any(AbortSignal), { privateRepo: false, refresh: false },
);
expect(mocks.resolve).toHaveBeenLastCalledWith('<h1>Fetched current file</h1>',
expect.objectContaining({ repoRef: expect.objectContaining({ path: 'second.html' }) }));
Expand Down Expand Up @@ -251,3 +251,18 @@ describe('blob view choice', () => {
expect(mocks.resolve).not.toHaveBeenCalled();
});
});


it('Retry fetches fresh HTML at the pinned commit instead of reusing embedded source', async () => {
mocks.resolve.mockResolvedValueOnce({ ...result, resources: { ...result.resources, failed: 1 },
diagnostics: [{ level: 'error', code: 'resource-fetch-failed', message: 'Failed image' }] });
start(); await settle();
expect(mocks.fetch).not.toHaveBeenCalled();
expect(mocks.resolve.mock.calls[0][0]).toBe('<h1>first.html</h1>');
const retry = Array.from(document.querySelectorAll<HTMLButtonElement>('button')).find(b => b.textContent === 'Retry')!;
retry.click(); await vi.advanceTimersByTimeAsync(1); await settle();
expect(mocks.fetch).toHaveBeenCalledWith({ owner: 'acme', repo: 'reports', ref: oid, path: 'first.html' },
expect.any(AbortSignal), { privateRepo: false, refresh: true });
expect(mocks.resolve).toHaveBeenLastCalledWith('<h1>Fetched current file</h1>', expect.objectContaining({ refresh: true, repoRef: expect.objectContaining({ ref: oid }) }));
expect(Array.from(document.querySelectorAll('button')).some(b => b.textContent === 'Retry')).toBe(false);
});
18 changes: 18 additions & 0 deletions tests/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,3 +270,21 @@ describe('repository fetching', () => {
expect(cancel).toHaveBeenCalledTimes(1);
});
});


it('private Retry reloads only the exact session URL without public transport or credentials leakage', async () => {
vi.stubGlobal('location', new URL('https://github.com/private-owner/private-repo/blob/main/report.html'));
const fetchMock = vi.fn(async (input: RequestInfo | URL, _init?: RequestInit) => {
const response = new Response('private HTML', { headers: { 'content-type': 'text/plain' } });
Object.defineProperty(response, 'url', { value: String(input) });
return response;
});
globalThis.fetch = fetchMock as typeof fetch;
await fetchRepositoryFile(repoRef, new AbortController().signal, { privateRepo: true, refresh: true });
expect(fetchMock).toHaveBeenCalledTimes(1);
expect(fetchMock.mock.calls[0]).toEqual([buildGitHubSessionRawUrl(repoRef), expect.objectContaining({ cache: 'reload', credentials: 'same-origin', referrerPolicy: 'same-origin' })]);
expect(fetchMock.mock.calls[0][1]?.headers).toBeUndefined();
fetchMock.mockClear();
await fetchRepositoryFile(repoRef, new AbortController().signal, { privateRepo: true });
expect(fetchMock.mock.calls[0][1]?.cache).toBeUndefined();
});
Loading
Loading