hugo-PaperMod icon indicating copy to clipboard operation
hugo-PaperMod copied to clipboard

support relative paths for opengraph images

Open chkuendig opened this issue 1 year ago • 1 comments
trafficstars

What does this PR change? What problem does it solve?

This allows referencing images in page bundles without having to put the whole path into the front-matter (which would break if the page moves or gets a custom slug).

Was the change discussed in an issue or in the Discussions before?

This came up when opengraph template was originally implemented ( https://github.com/adityatelange/hugo-PaperMod/issues/30 and https://github.com/adityatelange/hugo-PaperMod/issues/13, especially https://github.com/adityatelange/hugo-PaperMod/issues/30#issuecomment-706725646 )

PR Checklist

  • [ ] ~This change adds/updates translations and I have used the template present here.~
  • [x] I have enabled maintainer edits for this PR.
  • [x] I have verified that the code works as described/as intended.
  • [ ] ~This change adds a Social Icon which has a permissive license to use it.~
  • [x] This change does not include any CDN resources/links.
  • [x] This change does not include any unrelated scripts such as bash and python scripts.
  • [ ] This change updates the overridden internal templates from HUGO's repository.

chkuendig avatar Jan 10 '24 21:01 chkuendig

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarqubecloud[bot] avatar Jan 10 '24 21:01 sonarqubecloud[bot]

Relative paths are not recommended to be used with OpenGraph images, they won't work. Give this a read https://ogp.me/#data_types

adityatelange avatar Mar 08 '24 05:03 adityatelange

I understand, but this isn't what this PR does. It in fact converts relative paths to absolute URLs. This is purely about about accepting relative locations for the assets.

This is already discussed in detail in the linked above issues (including comments from you).

chkuendig avatar Mar 08 '24 06:03 chkuendig

Oh my bad I didn't pay enough attention it seems. Those issues are very old and I don't remember much. If you have some free time, can you please share some examples maybe a post.md where we have front-matter? Thanks

adityatelange avatar Mar 08 '24 07:03 adityatelange

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

sonarqubecloud[bot] avatar Mar 08 '24 07:03 sonarqubecloud[bot]

Sure, no worries :)

To explain in detail what this PR does, let's take the following post from my own page: https://christian.kuendig.info/posts/2024-01-scummvm-part3/

with the PR this is the current situation:

  • fontmatter (with the image in the same page bundle folder):
     images:
       - scummvm-ft-demo-00003.png
    
  • output: (I haven't setup a baseURL which is why it's missing here )
    <meta property="og:image" content="/posts/2024-01-scummvm-part3/scummvm-ft-demo-00003.png"/>
    

Before this PR, the image URL would have been /scummvm-ft-demo-00003.png which would not work and 404. The only way to make this work currently is to set the full path in the font matter like, the following, but that is very unelegant and breaks if you move the page or it's custom slug at any point without adjusting the font matter.

images:
  - /posts/2024-01-scummvm-part3/scummvm-ft-demo-00003.png

This is also an issue with the Hugo internal templates, see for example https://discourse.gohugo.io/t/links-for-images-videos-audio-in-internal-templates-twitter-cards-and-opengraph/10464/4 (2018, maybe it's been fixed by now).

You mention in https://github.com/adityatelange/hugo-PaperMod/issues/13#issuecomment-705313991 that this can't be fixed in papermod as it's an issue with the internal hugo templates, but the discussion later mentions various potential fixes which is what this PR is based on:

  • current url generation:{{ . | absURL }} - this just converts the input string to an absolute url based on the baseURL, see https://gohugo.io/functions/urls/absurl/.

  • with this PR: index (findRE "^/.*" . 1) 0 | default (print $.RelPermalink .) | absURL which works as follows:

    • first checks if the string starts with a slash (i.e.an absolute path) and then passes the whole string if it does or nothing in case it doesn't (index (findRE "^/.*" . 1) 0).
    • if nothing was passed (i.e. a relative path), the next part default (print $.RelPermalink .) takes the original string and prepends the relative permalink of the current page to it.
    • and finally we pass it to absURL as before the PR to get the full absolute URL.

This means that absolute paths skip the middle stage and the whole thing works the same as before, only for paths starting without a slash (i.e. relative) the middle pipe prefixes the pages relative permalink.

chkuendig avatar Mar 08 '24 08:03 chkuendig

Understood, but are there any benefits of not setting a baseURL?

adityatelange avatar Mar 08 '24 10:03 adityatelange

No, thats just a personal thing I forgot to set and wanted to mention why its missing in the example above.

Doesnt matter when it comes to this PR and this PR doesnt change that as it still runs through absURL as it did before so the baseURL will still be added if its set. The PR is purely about being able to add a image to the fontmatter with a relative path instead of absolute.

chkuendig avatar Mar 08 '24 10:03 chkuendig

I am trying to figure this out but I am not getting it. With cover images we use

# content/blog/mypost/index.md
---
cover: { image: media/cover/secure-http-headers.jpeg, relative: true }
---

Image is at content/blog/mypost/media/cover/secure-http-headers.jpeg

and it links well without setting the full path here.

https://github.com/adityatelange/hugo-PaperMod/blob/b5f7118d826e663bfe76f56eba2baa028a384325/layouts/partials/templates/opengraph.html#L5-L10

adityatelange avatar Mar 08 '24 11:03 adityatelange

Rest of the code is from Hugo templates unmodified.

https://github.com/gohugoio/hugo/blob/cb98e9061b3d62b6f1e635a6cf9a8be57dc3f56c/tpl/tplimpl/embedded/templates/opengraph.html

adityatelange avatar Mar 08 '24 11:03 adityatelange

There was some development on this https://github.com/gohugoio/hugo/pull/11688#issuecomment-1835416580

and I think https://github.com/gohugoio/hugo/commit/14d85ec136413dcfc96ad8e4d31633f8f9cbf410 fixes the issue?

adityatelange avatar Mar 08 '24 11:03 adityatelange

Actually that works when using a cover image, but if I don't want to use a cover image but still have an opengraph image, there's a backup path to set it a few lines after: https://github.com/adityatelange/hugo-PaperMod/blob/b5f7118d826e663bfe76f56eba2baa028a384325/layouts/partials/templates/opengraph.html#L14

This one unfortunately doesn't have the check for a relative path. This is also broken in the original hugo template - which is what I mentioned in the previous comment.

I guess an alternative solution would be to handle this the same way as the cover image (i.e. introduce a separate property to specify whether it's a relative path), but that wouldn't be backward compatible (or we'd still have to add yet another path to check if the image is a object/dict or a string).

Edit/PS: Just saw your latest comment now, I'll check this out now...

chkuendig avatar Mar 08 '24 11:03 chkuendig

If that fix works, we can clone https://github.com/gohugoio/hugo/commit/14d85ec136413dcfc96ad8e4d31633f8f9cbf410#diff-6be91eea11ef02863538c2b6698248757ad498326c234fe7c085c134c9a4245a to layouts/partials/templates/_funcs/get-page-images.html in PaperMod and pick remaining changes for opengraph and twitter_cards

  • https://github.com/gohugoio/hugo/commit/14d85ec136413dcfc96ad8e4d31633f8f9cbf410#diff-fa08dcd44528f7a509376d58270909ca26c568e1b51e4d6f0b7a77f8cc32e5cd
  • https://github.com/gohugoio/hugo/commit/14d85ec136413dcfc96ad8e4d31633f8f9cbf410#diff-5b97abb8773a42aee840ae8559e2961cef2412c3065e2d85778eb06868d74297

And keep the templates from hugo in sync with papermod as always.

adityatelange avatar Mar 08 '24 11:03 adityatelange

Great catch, I must have missed this (or it wasnt yet done when I looked into it originally).

I think this might solve the issue. Ill check and if it does close this PR and create a superseeding one with the upstream changes if thats okay? I guess we’d have to copy everything to make sure this also works on older versions of hugo, right?

On Fri, Mar 8, 2024, at 12:44 PM, Aditya Telange wrote:

If that fix works, we can @.***#diff-6be91eea11ef02863538c2b6698248757ad498326c234fe7c085c134c9a4245a to layouts/partials/templates/_funcs/get-page-images.html in PaperMod and pick remaining changes for opengraph and twitter_cards

@.#diff-fa08dcd44528f7a509376d58270909ca26c568e1b51e4d6f0b7a77f8cc32e5cd • @.#diff-5b97abb8773a42aee840ae8559e2961cef2412c3065e2d85778eb06868d74297

— Reply to this email directly, view it on GitHub https://github.com/adityatelange/hugo-PaperMod/pull/1389#issuecomment-1985551065, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWZGWGCW3KVL4K6SURBV3YXGQBBAVCNFSM6AAAAABBVOXZYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBVGU2TCMBWGU. You are receiving this because you authored the thread.Message ID: @.***>

chkuendig avatar Mar 08 '24 11:03 chkuendig

Great catch, I must have missed this (or it wasnt yet done when I looked into it originally). I think this might solve the issue. Ill check and if it does close this PR and create a superseeding one with the upstream changes if thats okay? I guess we’d have to copy everything to make sure this also works on older versions of hugo, right?

Yeah, I would have pulled this patch at some point anyways. If you create a PR make sure to add proper authorship for the commits, lets give credits to the person who wrote it :)

adityatelange avatar Mar 08 '24 12:03 adityatelange

Closing this as the changes from upstream are now merged: adityatelange/hugo-PaperMod@df330a05b5b3fe4a389982186a9bfd9ea8fc8de0.

chkuendig avatar Mar 11 '24 15:03 chkuendig