Skip to content
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.

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

promote local media assets to file attachments #250

Merged
merged 5 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions src/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {relativeUrl, resolvePath} from "./url.js";
// A path is local if it doesn’t go outside the the root.
export function getLocalPath(sourcePath: string, name: string): string | null {
if (/^\w+:/.test(name)) return null; // URL
if (name.startsWith("#")) return null; // anchor tag
const path = resolvePath(sourcePath, name);
if (path.startsWith("../")) return null; // goes above root
return path;
Expand Down
59 changes: 50 additions & 9 deletions src/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {computeHash} from "./hash.js";
import {parseInfo} from "./info.js";
import {type FileReference, type ImportReference, type Transpile, transpileJavaScript} from "./javascript.js";
import {transpileTag} from "./tag.js";
import {relativeUrl, resolvePath} from "./url.js";
import {resolvePath} from "./url.js";

export interface ReadMarkdownResult {
contents: string;
Expand Down Expand Up @@ -316,22 +316,63 @@ function renderIntoPieces(renderer: Renderer, root: string, sourcePath: string):
}
let result = "";
for (const piece of context.pieces) {
result += piece.html = normalizePieceHtml(piece.html, root, sourcePath, context);
result += piece.html = normalizePieceHtml(piece.html, sourcePath, context);
}
return result;
};
}

function normalizePieceHtml(html: string, root: string, sourcePath: string, context: ParseContext): string {
const SUPPORTED_PROPERTIES: readonly {query: string; src: "href" | "src" | "srcset"}[] = Object.freeze([
{query: "img[src]", src: "src"},
{query: "img[srcset]", src: "srcset"},
{query: "picture source[srcset]", src: "srcset"},
{query: "video[src]", src: "src"},
{query: "video source[src]", src: "src"},
{query: "audio[src]", src: "src"},
{query: "audio source[src]", src: "src"},
{query: "link[href]", src: "href"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I purposely didn't add a[href] because we already link to local .md files. See test case:
https://github.com/observablehq/cli/blob/67ebf20b3c1b993f9fc664fd8cd827353d371b65/test/input/build/config/index.md?plain=1#L3-L4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question of a[href] highlights a small flaw in our logic.

Here are three cases:

  • [another page](page) — the page is served from page.md, it's not a file attachment.
  • [view source](file.md) — file.md should be a file attachment
  • <a href="earthquakes.csv" download>download dataset</a> — data.csv should be a file attachment

Currently we base our choice on the tag: in the current approach audio, video, link… are never pointing to pages, and thus indicate a file attachment, and conversely a always indicates a page and never a file attachment.

But shouldn't this be associated to the URL routing instead? If the URL is routed to a md source (or to a static asset #169), it's not a file attachment; otherwise it's a (potential) file attachment.

That new logic could be applied to all the tags. Of course in practice an audio[src] will never be a page (since all pages generate HTML, which are not suitable for audio); but a link[href] can be a page — e.g., <link rel="next"> [ref].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Note that this issue will be solved with #257.)

Fil marked this conversation as resolved.
Show resolved Hide resolved
]);
export function normalizePieceHtml(html: string, sourcePath: string, context: ParseContext): string {
const {document} = parseHTML(html);

// Extracting references to files (such as from linked stylesheets).
for (const element of document.querySelectorAll("link[href]") as any as Iterable<Element>) {
const href = element.getAttribute("href")!;
const path = getLocalPath(sourcePath, href);
if (path) {
context.files.push(fileReference(href, sourcePath));
element.setAttribute("href", relativeUrl(sourcePath, join("_file", path)));
const filePaths = new Set<FileReference["path"]>();
for (const {query, src} of SUPPORTED_PROPERTIES) {
for (const element of document.querySelectorAll(query) as any as Iterable<Element>) {
if (src === "srcset") {
const srcset = element.getAttribute(src);
const paths =
srcset &&
srcset
.split(",")
.map((p) => {
const parts = p.trim().split(/\s+/);
const source = parts[0];
const path = getLocalPath(sourcePath, source);
if (path) {
const file = fileReference(source, sourcePath);
if (!filePaths.has(file.path)) {
filePaths.add(file.path);
context.files.push(file);
}
return `${file.path} ${parts.slice(1).join(" ")}`.trim();
}
return parts.join(" ");
})
.filter((p) => !!p);
if (paths && paths.length > 0) element.setAttribute(src, paths.join(", "));
} else {
const source = element.getAttribute(src);
const path = getLocalPath(sourcePath, source!);
if (path) {
const file = fileReference(source!, sourcePath);
if (!filePaths.has(file.path)) {
filePaths.add(file.path);
context.files.push(file);
}
element.setAttribute(src, file.path);
Fil marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

Expand Down
204 changes: 204 additions & 0 deletions test/promote-file-attachment-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
import assert from "node:assert";
import {normalizePieceHtml} from "../src/markdown.js";

const html = (strings, ...values) => String.raw({raw: strings}, ...values);
const mockContext = () => ({files: [], imports: [], pieces: [], startLine: 0, currentLine: 0});

describe("file attachments", () => {
describe("added", () => {
const sourcePath = "/attachments.md";

it("img[src]", () => {
const htmlStr = html`<img src="./test.png">`;
const expected = html`<img src="./_file/test.png">`;
const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, [
{
mimeType: "image/png",
name: "./test.png",
path: "./_file/test.png"
}
]);
});

it("img[srcset]", () => {
const htmlStr = html`
<img
srcset="small.jpg 480w, large.jpg 800w"
sizes="(max-width: 600px) 480px,
800px"
src="large.jpg"
alt="Image for testing"
/>
`;
const expected = html`
<img srcset="./_file/small.jpg 480w, ./_file/large.jpg 800w" sizes="(max-width: 600px) 480px,
800px" src="./_file/large.jpg" alt="Image for testing">
`;
const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, [
{
mimeType: "image/jpeg",
name: "large.jpg",
path: "./_file/large.jpg"
},
{
mimeType: "image/jpeg",
name: "small.jpg",
path: "./_file/small.jpg"
}
]);
});

it("video[src]", () => {
const htmlStr = html`<video src="observable.mov" controls>
Your browser doesn't support HTML video.
</video>`;
const expected = html`<video src="./_file/observable.mov" controls>
Your browser doesn't support HTML video.
</video>`;
const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, [
{
mimeType: "video/quicktime",
name: "observable.mov",
path: "./_file/observable.mov"
}
]);
});

it("video source[src]", () => {
const htmlStr = html`<video width="320" height="240" controls>
<source src="observable.mp4" type="video/mp4">
<source src="observable.mov" type="video/mov">
Your browser doesn't support HTML video.
</video>`;

const expected = html`<video width="320" height="240" controls>
<source src="./_file/observable.mp4" type="video/mp4">
<source src="./_file/observable.mov" type="video/mov">
Your browser doesn't support HTML video.
</video>`;

const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, [
{
mimeType: "video/mp4",
name: "observable.mp4",
path: "./_file/observable.mp4"
},
{
mimeType: "video/quicktime",
name: "observable.mov",
path: "./_file/observable.mov"
}
]);
});

it("picture source[srcset]", () => {
const htmlStr = html`<picture>
<source srcset="observable-logo-wide.png" media="(min-width: 600px)"/>
<img src="observable-logo-narrow.png" />
</picture>`;

const expected = html`<picture>
<source srcset="./_file/observable-logo-wide.png" media="(min-width: 600px)">
<img src="./_file/observable-logo-narrow.png">
</picture>`;

const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, [
{
mimeType: "image/png",
name: "observable-logo-narrow.png",
path: "./_file/observable-logo-narrow.png"
},
{
mimeType: "image/png",
name: "observable-logo-wide.png",
path: "./_file/observable-logo-wide.png"
}
]);
});
});

describe("not added", () => {
const sourcePath = "/attachments.md";

it("img[src] only adds local files", () => {
const htmlStr = html`<img src="https://upload.wikimedia.org/wikipedia/commons/thumb/0/0c/American_Shorthair.jpg/900px-American_Shorthair.jpg">`;
const expected = html`<img src="https://upload.wikimedia.org/wikipedia/commons/thumb/0/0c/American_Shorthair.jpg/900px-American_Shorthair.jpg">`;
const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, []);
});

it("img[srcset] only adds local files", () => {
const htmlStr = html`
<img
srcset="small.jpg 480w, https://upload.wikimedia.org/900px-American_Shorthair.jpg 900w"
sizes="(max-width: 600px) 480px, 900px"
src="https://upload.wikimedia.org/900px-American_Shorthair.jpg"
alt="Cat image for testing"
/>
`;
const expected = html`
<img srcset="./_file/small.jpg 480w, https://upload.wikimedia.org/900px-American_Shorthair.jpg 900w" sizes="(max-width: 600px) 480px, 900px" src="https://upload.wikimedia.org/900px-American_Shorthair.jpg" alt="Cat image for testing">
`;
const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, [
{
mimeType: "image/jpeg",
name: "small.jpg",
path: "./_file/small.jpg"
}
]);
});

it("video source[src] only adds local files", () => {
const htmlStr = html`<video width="320" height="240" controls>
<source src="https://www.youtube.com/watch?v=SsFyayu5csc" type="video/youtube"/>
<source src="observable.mov" type="video/mov">
Your browser doesn't support HTML video.
</video>`;

const expected = html`<video width="320" height="240" controls>
<source src="https://www.youtube.com/watch?v=SsFyayu5csc" type="video/youtube">
<source src="./_file/observable.mov" type="video/mov">
Your browser doesn't support HTML video.
</video>`;

const context = mockContext();
const actual = normalizePieceHtml(htmlStr, sourcePath, context);

assert.equal(actual, expected);
assert.deepEqual(context.files, [
{
mimeType: "video/quicktime",
name: "observable.mov",
path: "./_file/observable.mov"
}
]);
});
});
});