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.

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

Jump to bottom

Inter font paths are absolute #17575

Closed
rkfg opened this issue Jun 7, 2021 · 2 comments · May be fixed by #20040
Closed

Inter font paths are absolute #17575

rkfg opened this issue Jun 7, 2021 · 2 comments · May be fixed by #20040
Labels
A-Packaging Packaging, signing, releasing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@rkfg
Copy link
Contributor

rkfg commented Jun 7, 2021

Currently, paths to the Inter fonts are relative to the website root. It's not a problem if Element is hosted on its own domain (like https://develop.element.io where these fonts are loaded just fine). But if it's hosted under a custom path like https://example.com/element/ these fonts can't be loaded. All other resources have valid paths, only these fonts are broken. It can be easily fixed with this patch:

diff --git a/src/vector/index.html b/src/vector/index.html
index 061d249e..7921d37f 100644
--- a/src/vector/index.html
+++ b/src/vector/index.html
@@ -54,7 +54,7 @@
         var tag = htmlWebpackPlugin.tags.headTags[i];
         var path = tag.attributes && tag.attributes.href;
         if (path.indexOf("Inter") !== -1) { %>
-            <link rel="preload" as="font" href="<%= path %>" crossorigin="anonymous"/>
+            <link rel="preload" as="font" href=".<%= path %>" crossorigin="anonymous"/>
         <% }
     } %>

So that insteal of <link rel="preload" as="font" href="/fonts/Inter/Inter-SemiBoldItalic.10a60d8.woff2" crossorigin="anonymous"/> it would be <link rel="preload" as="font" href="./fonts/Inter/Inter-SemiBoldItalic.10a60d8.woff2" crossorigin="anonymous"/>. I'm not completely sure if this solution is good enough but it works for me. Maybe it's better to fix these paths somewhere else.
@rkfg rkfg added the T-Defect label Jun 7, 2021
@rkfg rkfg changed the title Inter fonts path are absolute Inter font paths are absolute Jun 7, 2021
@jryans jryans added A-Packaging Packaging, signing, releasing P3 S-Tolerable Low/no impact on users labels Jun 7, 2021
emikulic added a commit to emikulic/element-web that referenced this issue Dec 4, 2021
@emikulic
Make Inter font paths relative.
612d910 
Currently, paths look like /fonts/Inter which is fine if Element is installed in the wwwroot.
This change makes the paths relative (./fonts/Inter) which helps when it's not.

Credit goes to @rkfg.

Fixes element-hq#17575.
@emikulic emikulic mentioned this issue Dec 4, 2021
Open
@germain-gg germain-gg added O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist and removed P3 S-Tolerable Low/no impact on users labels Dec 8, 2021
@germain-gg
Copy link
Contributor

As discussed in #20040

I'm not convinced that this the approach I'd want us to take to fix this particular problem. Dabbling with the HtmlWebpackConfig sounds a bit more maintanable and will convey the intent a bit better.
It appears that the path or publicPath option seems to achieve what you're after.
I'd also be curious to understand why this is an issue for this scenario and not for the JavaScript bundles

@t3chguy
Copy link
Member

t3chguy commented Apr 18, 2023

This got fixed

@t3chguy t3chguy closed this as completed Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Packaging Packaging, signing, releasing O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make Inter font paths relative. emikulic/element-web
4 participants
@rkfg @jryans @germain-gg @t3chguy