Skip to content

Conversation

@gordonwoodhull
Copy link
Contributor

Summary

Fixes #13685

  • Fix issue where remote font URLs in brand.yml are incorrectly treated as file paths when the brand file is in a subfolder (like an extension)
  • Add isExternalPath helper to detect URLs (paths starting with a protocol like https:) and skip path joining for external font paths
  • Add ensureCssRegexMatches test assertion for verifying CSS content in supporting files

Test plan

  • Added smoke test that verifies:
    • CSS contains correct font URL (src:url("https://notofonts.github.io/...)
    • CSS does NOT contain broken path (_extensions/my-brand/https:)
  • Test passes locally

🤖 Generated with Claude Code

@posit-snyk-bot
Copy link
Collaborator

posit-snyk-bot commented Jan 16, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@gordonwoodhull
Copy link
Contributor Author

It hallucinated ensureCssRegexMatches, which I've often wanted, so I told it to ask deepwiki how to add the assertion.

Implementation is messy and IIUC just looks for CSS files in output directory - probably should be refined before merging.

@cderv
Copy link
Collaborator

cderv commented Jan 19, 2026

It hallucinated ensureCssRegexMatches, which I've often wanted, so I told it to ask deepwiki how to add the assertion.

This is interesting! I think we did use playwright for this kind of CSS verification until now. Mainly to be sure that the CSS applied in browser is the one expected. Or at least this is how I tried to use some CSS value check using playwright.

}:${styleString}wght@${weights}&display=${display}');`;
};

const isExternalPath = (path: string) => /^\w+:/.test(path);
Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW we have this

export function isHttpUrl(url: string) {
return /^https?:/i.test(url);
}

but yours is more generic. Also, if you need this one, you should probably add it in one of the core files as we already have several times

function isExternalPath(path: string) {
return /^\w+:/.test(path);
}

function isExternalPath(path: string) {
return /^\w+:/.test(path);
}

function isExternalPath(path: string) {
return /^\w+:/.test(path);
}

Maybe it is time to have this helper in one place.

gordonwoodhull and others added 4 commits January 19, 2026 17:02
Add a new test assertion function that checks regex patterns against
CSS files in a document's supporting files directory. This is useful
for verifying CSS content that is generated during rendering.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
When a brand.yml file with remote font URLs (e.g., https://...) is
located in a subfolder (like _extensions/my-brand/), the font URL
was incorrectly joined with the path prefix, resulting in broken
paths like "_extensions/my-brand/https:/...".

Add isExternalPath helper to detect URLs and skip path joining for
external font paths.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Move the duplicated isExternalPath function to a single location in
src/core/url.ts and update all call sites to import from there.

This addresses feedback from @cderv on PR #13901.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@gordonwoodhull gordonwoodhull force-pushed the fix/13685-remote-fonts-brand branch from fd3eb04 to bf48448 Compare January 19, 2026 22:02
Instead of globbing all CSS files in the support directory, parse the
HTML document to find `<link rel="stylesheet">` tags and only check
those specific CSS files. This is more targeted and accurate.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote fonts not recognised when using a brand extension

4 participants