fix(fonts): relative protocol urls - #13765
Conversation
🦋 Changeset detectedLatest commit: ebbfd22 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
the
pkg: astro
CodSpeed Performance ReportMerging #13765 will not alter performanceComparing Summary
|
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Can you create a e2e test (or integration test), so we test if the font is correctly loaded? I'm not sure a unit test is enough here
|
I approved this before seeing Ema's request for changes |
|
@ematipico I don't think a e2e test is needed here. The way the fonts are architected allow to test every part in isolation with unit tests. In this case, the bug is not that the font fetcher thinks the url is absolute when it's not (and fails). The bug is that the url it receives is incorrect. So I believe the test is actually fine since it makes sure this kind of url is handled earlier. Let me know what you think! |
3 participants
Changes
//), which are then considered absolute by the font fetcher and failshttpsTesting
Unit + manual
Docs
Changeset