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

Handle absurl replacement in inline style elements #5488

Closed
onedrawingperday opened this issue Dec 1, 2018 · 18 comments
Closed

Handle absurl replacement in inline style elements #5488

onedrawingperday opened this issue Dec 1, 2018 · 18 comments

Comments

@onedrawingperday
Copy link
Contributor

onedrawingperday commented Dec 1, 2018

Not sure if this is a bug but I am opening due to this comment: gohugoio/hugoDocs#676 (comment)

This behavior is documented in the Themes repository's README:

Common Permalink Issues

The demo of your theme will be available in a sub-directory of the Hugo Themes website.

  • You need to create absolute paths in the templates for the theme's assets, by using either the absURL function or .Permalink. Also make sure not to use a forward slash / in the beginning of a PATH, because Hugo will turn it into a relative URL and the absURL function will have no effect.

And yesterday I sent and merged a PR to amend the Hugo Docs that stated the opposite to the above.

Missing assets have been the most common issue that @digitalcraftsman and me encounter with theme submissions due to the use of .relURL or relative paths in theme templates.

If this could be somehow resolved so that both:

  • .Permalink and .RelPermalink
  • absURL and .relURL

output the correct URL for sites that are published in sub-directories (like the demos in Hugo Themes), it would be great.

@bep bep added this to the v0.53 milestone Dec 17, 2018
@bep
Copy link
Member

bep commented Dec 17, 2018

Do you have some concrete examples of "bad behaviour"? E.g. themes where this is/was an issue?

@onedrawingperday
Copy link
Contributor Author

onedrawingperday commented Dec 17, 2018

Here is an example from a theme that is still pending.

I had to send a (now merged) PR to that theme's repo because there were too many instances of relative URLs in the templates: 2-REC/hugo-myportfolio-theme#5

To test on your own with the Hugo Themes Build Script you would need to add that theme as a submodule and then from within its directory revert commit 2-REC/hugo-myportfolio-theme@1df8a11

Also here is the diff of what got "fixed" with the above PR: 2-REC/hugo-myportfolio-theme@9c290b5


Furthermore, please note that this behavior has been going on since way before I started reviewing themes in the Hugo Themes GitHub repository. The original guidance regarding the need to use absolute URLs for theme assets is by @digitalcraftsman

Also as stated in this issue's title this behavior is present not just in the demos on the Hugo Themes website but also it exists in all Hugo projects that are hosted in subdirectories.

There was also another submission the other day and the theme author had completely rewritten Hugo generated permalinks on his own "due to the limitation of Hugo not providing truly relative links". See the author's comment here: gohugoio/hugoThemes#515 (comment)

Kind of unrelated sidenote: the review of the Midnight theme submission was one of the hardest and more time consuming ones that I ever did due to all the permalink complexity.

@bep
Copy link
Member

bep commented Dec 17, 2018

There are some weird constructs in the theme you sent as an example ({{ .Permalink | relURL }} etc. makes no sense to me).

Skimming that change set, I can say that there is one known issue/limitation:

 style="background-image: url('{{ (printf "images/%s" .) | relURL }}');"

The above does not (currently) work with the combination of sub-path in baseURL + canonifyURLs, reason being that we don't do "canonify URL replacements inside style tags".

@onedrawingperday
Copy link
Contributor Author

onedrawingperday commented Dec 17, 2018

There are some weird constructs in the theme you sent as an example ({{ .Permalink | relURL }} etc. makes no sense to me).

Yes I forgot about that weirdness. Ok.

Here is another example.

This is another PR that I sent to the Hugo Shopping Product Catalogue theme:

kishaningithub/hugo-shopping-product-catalogue-simple@78c8642

Note how URLs that start with a forward slash generate links that are relative to the root of a project and therefore are incorrect when the project is published in a subdirectory.

@bep
Copy link
Member

bep commented Dec 17, 2018

Note how URLs that start with a forward slash generate links that are relative to the root of a project and therefore are incorrect when the project is published in a subdirectory.

The last example is constructing URLs from site params.

<a href="/foo">FOO</a>

The above URL points to the host root; Hugo cannot guess that the above should point to ... something else.

@onedrawingperday
Copy link
Contributor Author

onedrawingperday commented Dec 17, 2018

And finally here is another example that illustrates the wrong URLs outputted by relURL

This is another PR that I had to send due to the complexity of that theme (I have highlighted the diff of layouts/partials/head-theme.html for you to look at:

https://github.com/cshoredaniel/hugo-oldnew-mashup/pull/1/commits/cf823982cd8667763d683f2680a10681f9260ba0#diff-7a090ed8a787a737a667e3b902fc6371L12

P.S. There are plenty of examples the ones that I posted above are the most recent theme submissions from the top of my head, if you need more let me know and I'll post more links. (Also note that each submitted Hugo Theme has varying degrees of complexity)

@bep
Copy link
Member

bep commented Dec 17, 2018

Given your last example:

<link rel="stylesheet" href="{{ "/css/base.css" | relURL}}"/>

The above behaves fine in my tests.

{{ if $.Site.BaseURL }}<base href="{{ $.Site.BaseURL }}">{{ end }}

I suspect it is the above construct that messes with the URLs (in the browser that is) in the last example.

@onedrawingperday
Copy link
Contributor Author

onedrawingperday commented Dec 17, 2018

{{ if $.Site.BaseURL }}<base href="{{ $.Site.BaseURL }}">{{ end }}

Ah... I hadn't noticed the above. Ok, as I said each theme submission has varying degrees of complexity and I don't remember everything.

More examples about relURL here:
shaform/hugo-theme-den@cf7b65e

Tazeg/hugo-blog-jeffprod@4487419
In the above commit please note the comment by @digitalcraftsman

sakibccr/pristine@e3c2189
This commit -I think- that it best illustrates the problem.

Hope these examples are better.

@bep
Copy link
Member

bep commented Dec 17, 2018

<script src={{ "/js/main.js" | relURL }}></script>

See #5529

Note that the above could be written as:

<script src="{{ "/js/main.js" | relURL }}"></script>

And it would work fine. I had to look in the HTML 5 spec to confirm that unqouted attributes are allowed (they are), but I'm so used to putting quotes around them that I never thought about it.

Also note that the reason why we do these replacements is because of the canonifyURLs=true.

@bep
Copy link
Member

bep commented Dec 17, 2018

I have looked at the examples, and so far I see 2 cases where Hugo could improve with the combo of canonifyURLs=true and sub-path in baseURL:

  1. "style="background-image: url('{{ (printf "images/%s" .) | relURL }}');" (i.e. URLs inside style tags; this is a known issue)
  2. Support unquoted URLs in canonifyURLs replacer #5529

Is that a correct summary? @onedrawingperday @digitalcraftsman

@onedrawingperday
Copy link
Contributor Author

As far as I can tell your summary seems fine. Unless of course @digitalcraftsman has more examples.

Thanks for looking into this @bep

@digitalcraftsman
Copy link
Member

Is that a correct summary? @onedrawingperday @digitalcraftsman

This should cover all situations that occur to me spontaneously or have already been mentioned

@bep
Copy link
Member

bep commented Dec 17, 2018

OK, but quote your relative URLs and use absolute URLs in style tags doesn't sound like a too hard rule to follow for me. I will see if I find time to fix these.

@onedrawingperday
Copy link
Contributor Author

OK, but quote your relative URLs and use absolute URLs in style tags doesn't sound like a too hard rule to follow for me. I will see if I find time to fix these.

No it isn't.

I will send a PR to amend the README for the themes repo and another one for the Docs.

P.S. I removed myself from the Hugo Web team sometime ago. I'm still part of the organization but I don't belong in any team, so I no longer have merging rights in the Docs.

@bep I will tag you once I send the Docs PR so that you can review it and merge it.

@bep bep modified the milestones: v0.53, v0.54 Dec 19, 2018
@bep bep changed the title Sites published in sub-directories need absolute links Handle absurl replacement in inline style elements Dec 19, 2018
@onedrawingperday
Copy link
Contributor Author

onedrawingperday commented Dec 22, 2018

Right.

So since #5529 (transform/urlreplacers: Support unquoted URLs in canonifyURLs replacer) was resolved with commit f7691fe

And

This very issue has been renamed to: Handle absurl replacement in inline style elements

The guidance at: https://github.com/gohugoio/hugoThemes#common-permalink-issues needs to reflect the above. (PR coming soon).

@bep
Copy link
Member

bep commented Dec 22, 2018

@onedrawingperday correct. I may revisit the "inline style" problem later -- but that is a harder nut to crack.

@bep bep modified the milestones: v0.54, v0.55 Dec 26, 2018
bep pushed a commit to gohugoio/hugoDocs that referenced this issue Jan 3, 2019
See gohugoio/hugo#5488 (comment) and gohugoio/hugo#5488 (comment) for the rationale of this PR.

@bep Feel free to reword if you think, this could be improved.
@bep bep modified the milestones: v0.84, v0.85 Jun 18, 2021
@bep bep modified the milestones: v0.85, v0.86 Jul 5, 2021
@bep bep modified the milestones: v0.86, v0.87, v0.88 Jul 26, 2021
@bep bep modified the milestones: v0.88, v0.89 Sep 2, 2021
@bep bep modified the milestones: v0.89, v0.90 Nov 2, 2021
@bep bep modified the milestones: v0.90, v0.91.0 Dec 8, 2021
@bep bep modified the milestones: v0.91.0, v0.92.0 Dec 22, 2021
@bep bep modified the milestones: v0.92.0, v0.93.0 Jan 12, 2022
@bep bep modified the milestones: v0.93.0, v0.94.0 Mar 1, 2022
@bep bep modified the milestones: v0.94.0, v0.95.0, v0.96.0 Mar 9, 2022
@bep bep modified the milestones: v0.96.0, v0.97.0 Mar 24, 2022
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants