Skip to content

♻️ Refactor static site screenshot naming to use properties #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Robdel12 merged 2 commits into from
Oct 18, 2025
Merged

♻️ Refactor static site screenshot naming to use properties #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0185b43
♻️ Refactor static site screenshot naming to use properties
Robdel12 Oct 18, 2025
07aa4bb
✅ Add edge case tests for path sanitization
Robdel12 Oct 18, 2025
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
35 changes: 27 additions & 8 deletions clients/static-site/src/screenshot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ try {
}

/**
* Generate screenshot name from page and viewport
* Format: "path/to/page@viewportName"
* Generate screenshot name from page path
* Viewport info goes in properties for grouping
* @param {Object} page - Page object with path property
* @param {Object} viewport - Viewport object with name
* @returns {string} Screenshot name
*/
export function generateScreenshotName(page, viewport) {
export function generateScreenshotName(page) {
let { path } = page;
let viewportName = viewport.name;

// Remove leading slash for cleaner names
let cleanPath = path.startsWith('/') ? path.slice(1) : path;
Expand All @@ -34,7 +32,27 @@ export function generateScreenshotName(page, viewport) {
cleanPath = 'index';
}

return `${cleanPath}@${viewportName}`;
// Replace slashes and backslashes with hyphens to create valid filename
cleanPath = cleanPath.replace(/[/\\]/g, '-');

// Replace double dots (path traversal sequences) with single dots
cleanPath = cleanPath.replace(/\.\./g, '.');

return cleanPath;
}

/**
* Generate screenshot properties from viewport
* Properties are used by Vizzly for grouping and identification
* @param {Object} viewport - Viewport object with name, width, height
* @returns {Object} Screenshot properties
*/
export function generateScreenshotProperties(viewport) {
return {
viewport: viewport.name,
viewportWidth: viewport.width,
viewportHeight: viewport.height,
};
}

/**
Expand Down Expand Up @@ -70,8 +88,9 @@ export async function captureAndSendScreenshot(
viewport,
screenshotOptions = {}
) {
let name = generateScreenshotName(pageObj, viewport);
let name = generateScreenshotName(pageObj);
let properties = generateScreenshotProperties(viewport);
let screenshot = await captureScreenshot(page, screenshotOptions);

await vizzlyScreenshot(name, screenshot);
await vizzlyScreenshot(name, screenshot, { properties });
}
134 changes: 102 additions & 32 deletions clients/static-site/tests/screenshot.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,62 +3,132 @@
*/

import { describe, it, expect } from 'vitest';
import { generateScreenshotName } from '../src/screenshot.js';
import {
generateScreenshotName,
generateScreenshotProperties,
} from '../src/screenshot.js';

describe('generateScreenshotName', () => {
it('should generate correct screenshot name', () => {
it('should generate name from page path', () => {
let page = { path: '/about' };
let viewport = { name: 'mobile' };

let name = generateScreenshotName(page, viewport);

expect(name).toBe('about@mobile');
let name = generateScreenshotName(page);
expect(name).toBe('about');
});

it('should handle root path', () => {
let page = { path: '/' };
let viewport = { name: 'desktop' };

let name = generateScreenshotName(page, viewport);

expect(name).toBe('index@desktop');
let name = generateScreenshotName(page);
expect(name).toBe('index');
});

it('should handle nested paths', () => {
it('should replace slashes with hyphens', () => {
let page = { path: '/blog/post-1' };
let viewport = { name: 'tablet' };
let name = generateScreenshotName(page);
expect(name).toBe('blog-post-1');
expect(name).not.toContain('/');
});

let name = generateScreenshotName(page, viewport);
it('should handle nested paths', () => {
let page = { path: '/configuration/billing' };
let name = generateScreenshotName(page);
expect(name).toBe('configuration-billing');
});

expect(name).toBe('blog/post-1@tablet');
it('should handle deeply nested paths', () => {
let page = { path: '/api/v1/users/settings' };
let name = generateScreenshotName(page);
expect(name).toBe('api-v1-users-settings');
});

it('should handle paths without leading slash', () => {
let page = { path: 'docs/guide' };
let viewport = { name: 'mobile' };

let name = generateScreenshotName(page, viewport);

expect(name).toBe('docs/guide@mobile');
let name = generateScreenshotName(page);
expect(name).toBe('docs-guide');
});

it('should handle empty path', () => {
let page = { path: '' };
let viewport = { name: 'desktop' };
let name = generateScreenshotName(page);
expect(name).toBe('index');
});

let name = generateScreenshotName(page, viewport);
it('should handle backslashes', () => {
let page = { path: 'foo\\bar' };
let name = generateScreenshotName(page);
expect(name).toBe('foo-bar');
expect(name).not.toContain('\\');
});

it('should handle path traversal attempts', () => {
let testCases = [
{ path: '../../etc/passwd', expected: '.-.-etc-passwd' }, // .. becomes .
{ path: '../../../sensitive', expected: '.-.-.-sensitive' }, // .. becomes ., extra / becomes -
{ path: '/path/../secret', expected: 'path-.-secret' }, // .. becomes .
];

for (let testCase of testCases) {
let page = { path: testCase.path };
let name = generateScreenshotName(page);
expect(name).toBe(testCase.expected);
// Ensure no path traversal sequences remain
expect(name).not.toContain('..');
// Ensure no unescaped slashes remain
expect(name).not.toContain('/');
expect(name).not.toContain('\\');
}
});

expect(name).toBe('index@desktop');
it('should handle triple dots', () => {
let page = { path: '/normal/.../path' };
let name = generateScreenshotName(page);
// Triple dots contain .., which gets replaced: ... -> .
expect(name).toBe('normal-..-path');
expect(name).not.toContain('...');
});

it('should handle trailing slashes', () => {
let page = { path: '/about/' };
let name = generateScreenshotName(page);
expect(name).toBe('about-');
});
});

describe('generateScreenshotProperties', () => {
it('should generate properties with viewport info', () => {
let viewport = { name: 'mobile', width: 375, height: 667 };
let properties = generateScreenshotProperties(viewport);

expect(properties).toEqual({
viewport: 'mobile',
viewportWidth: 375,
viewportHeight: 667,
});
});

it('should include viewport dimensions', () => {
let viewport1 = { name: 'mobile', width: 375, height: 667 };
let viewport2 = { name: 'desktop', width: 1920, height: 1080 };
let viewport3 = { name: 'tablet', width: 768, height: 1024 };

let props1 = generateScreenshotProperties(viewport1);
let props2 = generateScreenshotProperties(viewport2);
let props3 = generateScreenshotProperties(viewport3);

expect(props1.viewportWidth).toBe(375);
expect(props1.viewportHeight).toBe(667);

expect(props2.viewportWidth).toBe(1920);
expect(props2.viewportHeight).toBe(1080);

expect(props3.viewportWidth).toBe(768);
expect(props3.viewportHeight).toBe(1024);
});

it('should handle different viewport names', () => {
let page = { path: '/pricing' };
let viewport1 = { name: 'mobile' };
let viewport2 = { name: 'desktop' };
let viewport3 = { name: 'tablet' };

expect(generateScreenshotName(page, viewport1)).toBe('pricing@mobile');
expect(generateScreenshotName(page, viewport2)).toBe('pricing@desktop');
expect(generateScreenshotName(page, viewport3)).toBe('pricing@tablet');
let viewport1 = { name: 'mobile', width: 375, height: 667 };
let viewport2 = { name: 'desktop', width: 1920, height: 1080 };

expect(generateScreenshotProperties(viewport1).viewport).toBe('mobile');
expect(generateScreenshotProperties(viewport2).viewport).toBe('desktop');
});
});