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

Output format (AMP) render hooks aren't getting used #7160

Closed
ct-martin opened this issue Apr 11, 2020 · 19 comments
Closed

Output format (AMP) render hooks aren't getting used #7160

ct-martin opened this issue Apr 11, 2020 · 19 comments

Comments

@ct-martin
Copy link

What version of Hugo are you using (hugo version)?

$ hugo version
Hugo Static Site Generator v0.68.3/extended linux/amd64 BuildDate: unknown

Does this issue reproduce with the latest release?

Yes, CI pipeline is running

$ hugo version
Hugo Static Site Generator v0.69.0-4205844B/extended linux/amd64 BuildDate: 2020-04-10T09:16:58Z

Expected Behavior

An <amp-img> would appear on the page as given in render-image.amp.html

Actual Behavior

An <img> appears on the page using the template in render-image.html

Misc.

Code: https://github.com/ct-martin/food/ (note that theme is a git submodule)

Forum thread: https://discourse.gohugo.io/t/markdown-render-hook-image-for-amp-template-not-working/23525/

@BorisWilhelms
Copy link

I am also running into this issue

$ hugo version
Hugo Static Site Generator v0.73.0-428907CC linux/amd64 BuildDate: 2020-06-23T16:30:43Z

@bep
Copy link
Member

bep commented Jun 24, 2020

Hmm...

@bep bep added this to the v0.74 milestone Jun 24, 2020
@hollowaykeanho
Copy link

Bumped into this issue while enabling AMP alternative output for my theme.

@hollowaykeanho
Copy link

I discovered that this issue #7434 is related. When I explicitly saved my outputs.toml every time, the amp-img works auto-magically.

Please let me know how to further investigate (or close the other ticket if it is considered duplicated).

@bep bep modified the milestones: v0.74, v0.75 Jul 13, 2020
@jmooring
Copy link
Member

Please test again with 0.74. I suspect this was resolved with #7455.

@hollowaykeanho
Copy link

hollowaykeanho commented Jul 15, 2020

Test results with Hugo Static Site Generator v0.74.1-15163266/extended linux/amd64 BuildDate: 2020-07-13T19:02:45Z.

Direct run with v0.73 resources

Result: amp-img is rendered without needing workaround.

Purged v0.73 resources and run v0.74 from scratch

Result: Issue persist. Workaround from #7434 must be applied in order to make shortcode amp.html works.

Direct run with v0.74 resource

Result: Issue persist. Workaround from #7434 must be applied in order to make shortcode amp.html works.

EDIT: updated the wrong issue number.

@blackb1rd
Copy link

Just for information in the case this issue is fixed and may need for specific test.

When the page with output format AMP using shortcodes "youtube.amp.html" and "render-image.amp.html" together

Example content:

![](image.jpg)

{{< youtube w7Ft2ymGmfc >}}

The output format AMP will get the result correctly and issue #7160 is gone.

but if there is a image without shortcodes,

Example content:

![](image.jpg)

it will hit the issue #7160 .

@hollowaykeanho
Copy link

hollowaykeanho commented Sep 14, 2020

@bep , double confirmation:

  1. is the solution for this issue is not included in v.0.75.0 right?
  2. is there a plan to fix this problem (multiple outputs feature)?

If item 2 is not clear, I may drop the AMP implementations entirely from the theme module since the multiple output is not functioning properly at build level. =(

@blackb1rd
Copy link

This issue is not specific as AMP format, But it affect all format related with Custom Output Formats Feature.

Example config for Custom Output Formats:

[outputFormats]
  [outputFormats.AppFormat]
    baseName = "index"
    name = "app"
    isPlainText = false
    mediaType = "text/html"
    isHTML = true
    permalinkable = true
    noUgly = false
    path = "app"

[outputs]
  section = ["HTML", "APP"]
  page = ["HTML", "APP"]

Creating render hooks at layouts/_default/_markup/render-image.app.html and this template file is not map too.

@idarek
Copy link

idarek commented Nov 8, 2020

I tried to look at AMP output as well and can confirm that when page got YouTube iframe and youtube.amp.html than render-image.amp.html is used as well.

However its not fully up to that. Without render-image... file and set AMP output, does img shall still be converted to amp-img I think, and is not.

@hollowaykeanho
Copy link

hollowaykeanho commented Nov 14, 2020

Some investigation update hopefully can help:

When I use .Page.AlternativeOutputFormats to investigate an AMP page, it dumps the following output:

map[AMP:{amphtml {AMP text/amphtml index amphtml false true false false true 0} /en-us/components/note/index.amp.html //localhost:8080/en-us/components/note/index.amp.html}]

This indicates that Hugo fails to identify its multiple outputs format where it is supposed to list all other formats except AMP itself.

While the .Page.Permalink behaves weirdly such that it automatically trimmed the output format filename (using suffix mode):

File structure: content/en-us/components/note.md
Request: http://holloway-seraphim:8080/en-us/components/note/index.amp.html
Got: http://holloway-seraphim:8080/en-us/components/note/

Still no chance to find a working workaround since Hugo does not dump any additional user-level information like full request URL (forum URL: https://discourse.gohugo.io/t/how-to-get-requests-url-as-it-is/29392).

EDIT:

  1. The investigation above was done inside a short-code. this unexpected phenomenon happens in multiple output format short-code.
  2. If applies to block template (e.g. layouts/_default/single.*.html), the above works properly.

@blackb1rd
Copy link

I did not see maintainer activity on this issue, so I belive this issue will not be fixed soon(or never be fixed).

Then I created a new fork https://github.com/neohugo/neohugo, may be you could try. but it migth break the compatible with the hugo too.

it contains fixes related AMP and also optimize memory and maybe faster for large website building.

@hollowaykeanho
Copy link

hollowaykeanho commented Nov 14, 2020

I thought of forking out the source code but it can be burdening for the long run. At the moment, @bep and his team is working on debugging the recently added npm packages feature. I do not mind the wait.

I do hope maintainers can provide some attention to this issue as multiple outputs feature is completely blocked without workaround from theme developer POV. There is no way to go around permalink or getting a pure request URL for shortcode.

Prehaps can you upstream you workaround fixes for him to review?

@idarek
Copy link

idarek commented Nov 14, 2020

I did not see maintainer activity on this issue, so I belive this issue will not be fixed soon(or never be fixed).

Then I created a new fork https://github.com/neohugo/neohugo, may be you could try. but it migth break the compatible with the hugo too.

it contains fixes related AMP and also optimize memory and maybe faster for large website building.

It is probably somehow appreciated that you spend some time and implement changes in relation to this issue. I think, instead of forking Hugo is not a great idea for day-to-day use. For testing in finding a solution, yes. I think @bep and his team would be appreciated if you will rather share your finding and solution to this issue so they can implement this to the main batch.

This issue is not on somehow high priority, but every help counts.

@hollowaykeanho
Copy link

@idarek, are you familiar with the source code specifically at process mapping for shortcode? I do have strong Go background but I need a code navigator as a lighthouse.

The repository is large and it will take some times for me just to understand how things work before fixing any bugs.

@idarek
Copy link

idarek commented Nov 14, 2020

@idarek, are you familiar with the source code specifically at process mapping for shortcode? I do have a strong Go background but I need a code navigator as a lighthouse.

The repository is large and it will take some times for me just to understand how things work before fixing any bugs.

Sorry but no. Not a developer but a more rather self-taught person. Can support in testing etc.
Learning Hugo and usage of Go only for couple months soo far, and only in a matter of creating websites (experimenting with mine, to be able to use on other projects).

@blackb1rd
Copy link

I thought of forking out the source code but it can be burdening for the long run. At the moment, @bep and his team is working on debugging the recently added npm packages feature. I do not mind the wait.

I do hope maintainers can provide some attention to this issue as multiple outputs feature is completely blocked without workaround from theme developer POV. There is no way to go around permalink or getting a pure request URL for shortcode.

Prehaps can you upstream you workaround fixes for him to review?

You may take a look at PRs that I have been submit or other PR #3934 which is important feature first.

@jmooring
Copy link
Member

jmooring commented Mar 9, 2021

Fixed with #8310. Please close.

@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 Jan 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants