Skip to content

Handle relative links in <source srcset> #2976

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Handle relative links in <source srcset> #2976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion src/lib/converter/comments/textParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ function checkReference(data: TextParserData): RelativeLink | undefined {
}

/**
* Looks for `<a href="./relative">` and `<img src="./relative">`
* Looks for `<a href="./relative">`, `<img src="./relative">`, and `<source srcset="./relative">`
*/
function checkTagLink(data: TextParserData): RelativeLink | undefined {
const { pos, token } = data;
Expand All @@ -311,6 +311,11 @@ function checkTagLink(data: TextParserData): RelativeLink | undefined {
data.pos += 3;
return checkAttribute(data, "href");
}

if (token.text.startsWith("<source ", pos)) {
data.pos += 8;
return checkAttribute(data, "srcset");
Copy link
Author

Choose a reason for hiding this comment

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

The <source> tag supports:

Ideally we should support both, but I'm not familiar with how these functions work in the context of the parser. It seems that it mutates the data as it gets passed around, so I'm not sure how safe it is to e.g. store the original pos and call checkAttribute for src with the original pos after checking for srcset.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, the amount of random state that this code needs to parse around is very unfortunate; it's gotten worse since the original implementation because markdown is special... I think that should be fine here; but I really need to revisit this at some point.

Unfortunately, doing that isn't good enough. srcset isn't just one src, it's a potentially comma separated list with additional metadata (spec)

}
}

function checkAttribute(
Expand Down
32 changes: 32 additions & 0 deletions src/test/comments.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1636,6 +1636,38 @@ describe("Comment Parser", () => {
);
});

it("Recognizes HTML picture source srcset links", () => {
const comment = getComment(`/**
* <source media="(prefers-color-scheme: light)" srcset="./test.png" >
* <source media="(prefers-color-scheme: dark)" srcset="./test space.png"/>
* <source srcset="https://example.com/favicon.ico">
*/`);

equal(
comment.summary,
[
{ kind: "text", text: '<source media="(prefers-color-scheme: light)" srcset="' },
{
kind: "relative-link",
text: "./test.png",
target: 1 as FileId,
targetAnchor: undefined,
},
{ kind: "text", text: '" >\n<source media="(prefers-color-scheme: dark)" srcset="' },
{
kind: "relative-link",
text: "./test space.png",
target: 2 as FileId,
targetAnchor: undefined,
},
{
kind: "text",
text: '"/>\n<source srcset="https://example.com/favicon.ico">',
},
] satisfies CommentDisplayPart[],
);
});

it("Recognizes HTML anchor links", () => {
const comment = getComment(`/**
* <a data-foo="./path.txt" href="./test.png" >
Expand Down