figure shortcode produces unwanted space inside anchor element

ndim commented 3 years ago

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 commented 3 years ago

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

ndim commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

Will do, will take a bit, though.

ndim commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

github-actions[bot] commented 2 years 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.

gohugoio / hugo