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

figure shortcode produces unwanted space inside anchor element #8401

Closed
ndim opened this issue Apr 9, 2021 · 9 comments · Fixed by #8404
Closed

figure shortcode produces unwanted space inside anchor element #8401

ndim opened this issue Apr 9, 2021 · 9 comments · Fixed by #8404

Comments

@ndim
Copy link

ndim commented Apr 9, 2021

When used to generate a link like

{{% figure src="foo.png" link="foo.png" %}}

the figure shortcode produces unwanted whitespace inside the anchor tag, and that whitespace will be shown underlined in many cases, which looks confusing to say the least:

<figure><a href="foo.png">
    <img src="foo.png"/> </a>
</figure>

This applies to both the hugo-0.80.0-2.fc34.x86_64 package Fedora is shipping,

Hugo Static Site Generator v0.80.0/extended linux/amd64 BuildDate: unknown

both when using its builtin figure shortcode and when using today's master branch (cf the SHA in the link below) copied into themes/THEME/layouts/shortcodes/figure.html.

Taking a look at the implementation of the figure shortcode and playing around a bit, I found that wrapping the newlines and whitespace between the tag beginning the <a> element and the <img tag can be commented out with <!-- ... --> allowing to keep the existing indentation of the source, while stopping generating the unwanted whitespace:

<figure><a href="foo.png"><img src="foo.png"/></a>
</figure>

An alternative fix would be to "just" remove the whitespace from the source code, which also works, but that makes the code much more difficult to read.

No patch included, as I have not signed any CLA at this time.

@davidsneighbour
Copy link
Contributor

davidsneighbour commented Apr 9, 2021

call {{- % figure src="foo.png" link="foo.png" % -}}?

@ndim
Copy link
Author

ndim commented Apr 9, 2021

I have no idea what {{ - % % - }} would do (doing computers for 30 years, but new to hugo and Go), but using the builtin figure shortcode,

{{- % figure src="foo.png" link="foo.png" % -}}

produces

<p>{{- % figure src=&ldquo;foo.png&rdquo; link=&ldquo;foo.png&rdquo; % -}}</p>

which is neither a link anchor nor an image.

@ndim
Copy link
Author

ndim commented Apr 10, 2021

@jmooring All the checks in the patch (I have picked one at random)

"<figure><a href=\"/jump/here/on/clicking\">\n    <img src=\"/found/here\"/></a>\n</figure>"

still contain the problematic whitespace after the closing > of the opening <a> tag before the < of the <img element (one \n followed by several spaces).

@jmooring
Copy link
Member

Upon further review I believe this is a solution without a problem. See https://jsfiddle.net/rbw5gL6u/.

whitespace will be shown underlined in many cases

Please post an example, using the existing shortcode template, where this is true.

@ndim
Copy link
Author

ndim commented Apr 10, 2021

Will do, will take a bit, though.

@ndim
Copy link
Author

ndim commented Apr 11, 2021

TLDR

  • There are HTML pages whitespace between <a> and <img causes broken rendering (see image, code, and link below)
  • hugo's figure shortcode can produce HTML pages with such whitespace
  • hugo would be improved if its {{% figure ... %}} shortcode never produced such whitespace.
  • I can easily use my own layout/shortcode/ndim-figure.html which I have fixed to not produce such whitespace.
  • If the hugo team decide that hugo should keep on producing such whitespace, I can easily use {{% ndim-figure ... %}} to work around the problem indefinitely, so this does not prevent me from using hugo, just from using hugo's builtin figure shortcode.

The Elaboration

I have not been able to shrink this problem to a minimum working example hugo website yet. The following is a screenshot from a test page demonstrating the problem with my blog only partially migrated from jekyll/octopress to hugo. This partially migrated website is not public. Note the horizontal blue line at the left edge of the text column, just above the bottom image:

gnome-shell-screenshot-20EU10-shrunk

The HTML code being rendered is the following, where the only difference between the first image which is shown properly and the second image which has the weird horizontal line is the absence of whitespace before the <img tag in the first image:

<p>However, I had run out of heatshrink tubing, so I had to find another
way to keep blank contact surfaces from touching: I cut a 3x3 piece
from the corner of a piece of perfboard to make the connections:</p>
<figure class="center-img"><a href="layout-perfboard.png"><img src="layout-perfboard.png"
         alt="perfboard layout of headset adapter"/></a>
</figure>
<p>Actually, I put the headphone cable in a layer below and between the
microphone cable and the electrolytic cap, so the whole circuit fits
into a 12mm diameter hole drilled about 6cm deep:</p>
<figure class="center-img"><a href="layout-perfboard-physical.png">
    <img src="layout-perfboard-physical.png"
         alt="perfboard layout of headset adapter"/> </a>
</figure>
<p>(Photographs to follow)</p>

I have just manually edited the old blog page generated by jekyll/octopress (which I am in the middle moving over to hugo) to demonstrate the whitespace problem for anybody who wants to reproduce the weird underlined space rendering in their own browser: http://n-dimensional.de/blog/2015/04/20/cellphone-headset-on-pc-or-mini-mixer/ (that actually uses <div> instead of <figure>, but that makes no difference.)

But Why Exactly?

I remember always having had such rendering of whitespace betwheen <a> and <img> whenever images are used as anchor texts, ever since I have been first writing HTML around 1998 to 1999.

Weirdly though, my 2021 attempts to find a minimum working example demonstrating the problem by creating a new hugo demo website from scratch have not been able to reproduce the problem yet so far. And now I want to know why my old jekyll/octopress generated blog shows the problem, my far from complete port of my blog to hugo shows the problem, but a simple HTML file written from scratch does not.

Maybe the browser is selecting a strict rendering mode for one kind of page, but not for the other, and the whitespace rendering differs?

@jmooring
Copy link
Member

Your site behaves this way because of line 331 of http://n-dimensional.de/stylesheets/screen.css.

article a {
  white-space: pre-wrap;
}

https://developer.mozilla.org/en-US/docs/Web/CSS/white-space#values

pre-wrap
Sequences of white space are preserved.

@ndim
Copy link
Author

ndim commented Apr 12, 2021

Wow. Very good catch.

I am so sorry for

  • having a wrong impression that this was old browser behaviour
  • publicly perpetuating this wrong impression
  • most importantly: wasting your time with it

I cannot find where I got this impression from, but my brain appears to have rearranged its memories in a way such that to me it appeared to have always been the case that whitespace in HTML anchors outside image elements always leads to weird renderings.

I have traced the origin of the white-space: pre-wrap to the following SASS (or SCSS?) statement from compass, which I have committed in 2013 because either I actively thought wrapping long lines of continuous text was a good idea, or maybe because octopress (by Brandon Mathis, who is also listed as a Compass core team member) just had it activated for that or some other reason:

/* @extend this to force long lines of continuous text to wrap */
.force-wrap {
  white-space: -moz-pre-wrap;
  white-space: -pre-wrap;
  white-space: -o-pre-wrap;
  white-space: pre-wrap;
  word-wrap: break-word;
}

As it is, hugo's figure shortcode generates good and proper HTML for almost all use cases, except if someone happens to use the generated HTML with a CSS stylesheet which sets white-space to pre-wrap (or possibly even other values?).

Changing the figure shortcode to not generate the whitespace would make the generated HTML compatible with a larger variety of CSS stylesheets. There would be no disadvantage, except that changing the test cases and reviewing the pull request takes a bit of work.

I do not know how widely in use white-space values are on the general web, much less how widely in use they are amongst those who use website generators like hugo.

Anyway, FWIW, now that I have signed the CLA, this would be the patch with what I still think is a fix, but not of quite so broad an appeal I originally thought. (I had been unaware earlier that I can signing the CLA electronically and that it does not involve me hiring an attorney to figure out and snail-mail hundreds of pages of legalese.)

This patch does not involve any test code, however. For some reason, building hugo from source only works with warnings and "mage check" throws so many warnings even on an unmodified build that I have no idea where to start.

diff --git a/tpl/tplimpl/embedded/templates/shortcodes/figure.html b/tpl/tplimpl/embedded/templates/shortcodes/figure.html
index f81bdadf..ecabb286 100644
--- a/tpl/tplimpl/embedded/templates/shortcodes/figure.html
+++ b/tpl/tplimpl/embedded/templates/shortcodes/figure.html
@@ -1,14 +1,14 @@
 <figure{{ with .Get "class" }} class="{{ . }}"{{ end }}>
     {{- if .Get "link" -}}
         <a href="{{ .Get "link" }}"{{ with .Get "target" }} target="{{ . }}"{{ end }}{{ with .Get "rel" }} rel="{{ . }}"{{ end }}>
-    {{- end }}
+    {{- end -}}
     <img src="{{ .Get "src" }}"
          {{- if or (.Get "alt") (.Get "caption") }}
          alt="{{ with .Get "alt" }}{{ . }}{{ else }}{{ .Get "caption" | markdownify| plainify }}{{ end }}"
          {{- end -}}
          {{- with .Get "width" }} width="{{ . }}"{{ end -}}
          {{- with .Get "height" }} height="{{ . }}"{{ end -}}
-    /> <!-- Closing img tag -->
+    /><!-- Closing img tag -->
     {{- if .Get "link" }}</a>{{ end -}}
     {{- if or (or (.Get "title") (.Get "caption")) (.Get "attr") -}}
         <figcaption>

If you do not want to waste more time on this updating the test cases, I would offer to submit a pull request with updated test cases at some time in the future, whenever I get around to make hugo properly build from source here. This involves learning about the whole Go language and language ecosystem, though, so this might take some time.

@bep bep closed this as completed in #8404 Apr 15, 2021
bep pushed a commit that referenced this issue Apr 15, 2021
@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 21, 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.

3 participants