gohugoio / hugo

The world’s fastest framework for building websites.
https://gohugo.io
Apache License 2.0
76.2k stars 7.55k forks source link

Opengraph template locale- fallback to site's locale param #8296

Closed coliff closed 8 months ago

coliff commented 3 years ago

Currently the built-in Opengraph template only adds the <meta property="og:locale" content="" /> meta tag to a page if the locale param is in the frontmatter of the page. It'd be great if no param was specified in the frontmatter then it used the locale param in the sites config instead. It could make maintenance easier (if you have a 200-page site in a single language you only need to define the locale in the site config).

Maybe something like this?

{{- with .Params.locale }}<meta property="og:locale" content="{{ . }}" />
{{else}}
{{- with .Site.Params.locale }}<meta property="og:locale" content="{{ . }}" />
{{ end }}

cc @djatwood

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

davidsneighbour commented 3 years ago

This should be done because the Facebook documentation on that tag says "Defaults to en_US." - which is probably not what everyone wants as default if left empty. I would suggest:

{{- with .Params.locale }}<meta property="og:locale" content="{{ . }}" />
{{else}}
{{- with .Site.Params.locale }}<meta property="og:locale" content="{{ . }}" />
{{ else }}
<meta property="og:locale" content="en_US" />
{{ end }}
{{ end }}
coliff commented 3 years ago

I disagree. If, for example, a French-language site doesn't specify (forgets or doesn't know - or for whatever reason) a locale param on their site then they will specifically specify that their site is in American English. This would be bad. It'd be better to not specify it than specify the wrong value.

REF: https://ogp.me/ og:locale isn't a required tag - so it'd be ok to leave it out.

davidsneighbour commented 3 years ago

"og:locale - The locale these tags are marked up in. Of the format language_TERRITORY. Default is en_US."

I understand that as "if it's not set it is assumed en_US". Maybe add a warnf instead of the default tag so that people using that partial will know that something is going on? With a link to the resulting documentation or this issue here.

coliff commented 3 years ago

I don't think it should add locale unless the user specifies it on the page or in the site config. It's not a required param. If you check Hugo's homepage with https://developers.facebook.com/tools/debug/?q=https%3A%2F%2Fgohugo.io%2F - it doesn't have any errors or warnings about the locale not being set.

davidsneighbour commented 3 years ago

I am not saying you are wrong, I am saying if Hugos website would be Chinese, Facebook would think it's en_US due to not having a og:locale tag. If that template is touched why not make it perfect with a quick notice if it's not set up properly?

coliff commented 3 years ago

Maybe a site scanning for Opengraph tags would fallback to the lang attribute on the html element <html lang=""> tag? Not sure though... I still don't think Hugo should give any warnings about an optional parameter not being specified.

davidsneighbour commented 3 years ago

I am just suggesting ;) I am not using the internal templates myself - mostly because Hugo does create my site, not my HTML :) The thumbs up on the starting post still stands.

torrayne commented 3 years ago

So I looked over the current situation with multilingual mode. I believe the opengraph template has been left behind with all the updates to multilingual support. Open Graph wants the locale specified with as language_TERRITORY but Hugo can't even produce this format right now.

Using this config

languageCode = "en-US"
defaultContentLanguage = "en_US"

[languages]
    [languages.en_US]
        title = "English site title"
    [languages.es_ES]
        title = "Spanish site title"

You can then access page.Language

<!-- layouts/_default/baseof.html -->
<head>
<!-- This translation -->
{{ with .Language }}<meta property="og:locale" meta="{{ .Language.Lang }}">{{ end }}

<!-- Other translations -->
{{ range .Translations }}
<meta property="og:locale:alternate" content="{{ .Language.Lang }}" />
{{ end }}
</head>

.Language a language object that points to the language’s definition in the site config. .Language.Lang gives you the language code.

See docs .Language and .Language.Lang produce the same result but the documentation says to use the latter

Produces this result:

public
| en_US/
| | index.html containing no body
|
| en_us/
| | content...
|
| es_es/
| | content...

With the locale all lower case.

<meta content="es_es">
<meta property="og:locale:alternate" content="en_us">

I'm not sure what's the best way to go about fixing this. Hugo supports 2 different ways of translating content. Content directory content/locale/ and page.locale.md. Maybe adding a territory param to the language config would be the easiest option. But - and I don't know if this is something that happens in the real world - what about translating the same language but to different territories?


[languages]
  [languages.es]
    territory = "ES"
torrayne commented 3 years ago

Okay fooling around some more and I've come up with this solution.

# config.toml
defaultContentLanguage = "en"
[languages]
    [languages.en]
        title = "English Site Title"
        locale = "en_US"
    [languages.es]
        title = "Spanish Site Title"
        locale = "es_ES"
<!-- baseof.html or opengraph.html -->

{{ with .Language.Params.locale }}
<meta property="og:locale" content="{{ . }}">
{{ end }}

{{ range where .Translations "params" "locale" }}
<meta property="og:locale:alternate" content="{{ .Language.Params.locale }}" />
{{ end }}

Produces:

<meta property="og:locale" content="es_ES">
<meta property="og:locale:alternate" content="en_US">

This way it only adds languages with the locale specified.

bep commented 3 years ago

I think this should do:

{{ with .Param "locale" }}
<meta property="og:locale" content="{{ . }}">
{{ end }}

Also, we should probably also add Locale (if that's the name we go for, I think it's working?) on Language, so we could do:

{{ with .Language.Locale }}
<meta property="og:locale" content="{{ . }}">
{{ else }}
{{ with .Param "locale" }}
<meta property="og:locale" content="{{ . }}">
{{ end }}

We currently have Lang which many uses for this (which is basically the language key/name) set in config, but that may be different than the string you want in locale.

Or something.

torrayne commented 3 years ago

@coliff any chance to review the changes?

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help. If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open. If this is a feature request, and you feel that it is still relevant and valuable, please tell us why. This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

bep commented 2 years ago

Sorry for the delayed response.

I do agree that this needs to be fixed, and I do agree (as in the PR that was opened against this, #8306) that the configuration needs to live on Language. #8306 adds a new Language.Locale attribute. I think we have templates today that use the Lang value where you currently suggest using a new Locale, and we certainly have lots of Go code using Lang.

With #8306 we would get:

[languages]
    [languages.en]
        locale = "en_US"

==>

lang="en"
locale="en_US"

So, before we do this we need to determine if,

  1. Do we need a new field and is locale a fitting name?
  2. Why not use lang, the below TOML should work ?
  3. If we add locale should we then use that in every place where lang is currently used?
[languages]
    [languages.en_US]

/cc @jmooring

jmooring commented 2 years ago

Terminology

Term Example Notes
language en
region US Also known as a territory
locale en-US Also known as a language tag per RFC 5646

Overview

From a site author's perspective, Hugo's language key (e.g., the en in [languages.en]) currently has four primary roles:

  1. It is prepended to the path portion of the published site (e.g., https://example.org/en/articles/foo)
  2. It links the site with a translation file in the i18n directory. To properly pluralize a translation, the language key must match one of the languages in: https://github.com/nicksnyder/go-i18n/blob/main/v2/internal/plural/codegen/plurals.xml
  3. It is used to localize dates, numbers, percentages, and currencies. To properly localize these values, the language key must match one of the locales in: https://github.com/gohugoio/locales
  4. It is used to determine collation when sorting. This is relatively new, and I am not yet familiar with the implementation details. I imagine it is dependent on language, not locale, but I could be wrong.

When we read the language key from the site configuration, we convert it to lower case, and you do not have access to the original value from the templates.

If I want an English language site localized to England, Wales, and Scotland, I must set the language key to en-gb or en-GB (hyphens, not underscores).

In some configurations the language key is just the language (e.g., en) while in other configurations it is the locale (e.g., en-GB).

And at some point in the distant past we added .Site.LanguageCode, typically set in site configuration to something like en-us, en-US, en_US, etc. This isn't a language code; it's a locale. Currently, in the Hugo code base, I think it is only used twice:

Example configuration

defaultContentLanguage = 'en-gb'
defaultContentLanguageInSubdir = true

[languages.en-gb]
  contentDir = 'content/english-in-gb'
  languageCode = 'en_GB'  # locale
  languageName = 'English (GB)'
  weight = 1

[languages.en-us]
  contentDir = 'content/english-in-us'
  languageCode = 'en_US'  # locale
  languageName = 'English (US)'
  weight = 2

Note that this configuration requires two translation files: i18n/en-gb.toml and i18n/en-us.toml. This is not ideal because the files will be identical.

Recommendations

Now (this issue):

Later:

Much later:

bep commented 10 months ago

@coliff is this still unresolved?

github-actions[bot] commented 7 months ago

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.