Fix Plainify edge cases

This commit replaces the main part of `helpers.StripHTML` with Go's implementation in its html/template package.

It's a little slower, but correctness is more important:

```bash
BenchmarkStripHTMLOld-10    	  680316	      1764 ns/op	     728 B/op	       4 allocs/op
BenchmarkStripHTMLNew-10    	  384520	      3099 ns/op	    2089 B/op	      10 allocs/op
```

Fixes #9199
Fixes #9909
Closes #9410

diff --git a/helpers/content.go b/helpers/content.go
index 835663b76d2ca560eeafd1fe51e1cffb4b5ab20c..d04e34a07b887e8979730c38b15ef65d58a0116a 100644 (file)
--- a/helpers/content.go
+++ b/helpers/content.go
@@ -34,7 +34,6 @@ import (
 
        "github.com/gohugoio/hugo/markup"
 
-       bp "github.com/gohugoio/hugo/bufferpool"
        "github.com/gohugoio/hugo/config"
 )
 
@@ -104,45 +103,6 @@ func NewContentSpec(cfg config.Provider, logger loggers.Logger, contentFs afero.
        return spec, nil
 }
 
-var stripHTMLReplacer = strings.NewReplacer("\n", " ", "</p>", "\n", "<br>", "\n", "<br />", "\n")
-
-// StripHTML accepts a string, strips out all HTML tags and returns it.
-func StripHTML(s string) string {
-       // Shortcut strings with no tags in them
-       if !strings.ContainsAny(s, "<>") {
-               return s
-       }
-       s = stripHTMLReplacer.Replace(s)
-
-       // Walk through the string removing all tags
-       b := bp.GetBuffer()
-       defer bp.PutBuffer(b)
-       var inTag, isSpace, wasSpace bool
-       for _, r := range s {
-               if !inTag {
-                       isSpace = false
-               }
-
-               switch {
-               case r == '<':
-                       inTag = true
-               case r == '>':
-                       inTag = false
-               case unicode.IsSpace(r):
-                       isSpace = true
-                       fallthrough
-               default:
-                       if !inTag && (!isSpace || (isSpace && !wasSpace)) {
-                               b.WriteRune(r)
-                       }
-               }
-
-               wasSpace = isSpace
-
-       }
-       return b.String()
-}
-
 // stripEmptyNav strips out empty <nav> tags from content.
 func stripEmptyNav(in []byte) []byte {
        return bytes.Replace(in, []byte("<nav>\n</nav>\n\n"), []byte(``), -1)

diff --git a/helpers/content_test.go b/helpers/content_test.go
index 4b67b44f009cbfabdfb628e87d81b359e8c401e4..54b7ef3f955a6aeaacec6946c9d845c9fd5c64b9 100644 (file)
--- a/helpers/content_test.go
+++ b/helpers/content_test.go
@@ -52,44 +52,6 @@ func TestTrimShortHTML(t *testing.T) {
        }
 }
 
-func TestStripHTML(t *testing.T) {
-       type test struct {
-               input, expected string
-       }
-       data := []test{
-               {"<h1>strip h1 tag <h1>", "strip h1 tag "},
-               {"<p> strip p tag </p>", " strip p tag "},
-               {"</br> strip br<br>", " strip br\n"},
-               {"</br> strip br2<br />", " strip br2\n"},
-               {"This <strong>is</strong> a\nnewline", "This is a newline"},
-               {"No Tags", "No Tags"},
-               {`<p>Summary Next Line.
-<figure >
-
-        <img src="/not/real" />
-
-
-</figure>
-.
-More text here.</p>
-
-<p>Some more text</p>`, "Summary Next Line.  . More text here.\nSome more text\n"},
-       }
-       for i, d := range data {
-               output := StripHTML(d.input)
-               if d.expected != output {
-                       t.Errorf("Test %d failed. Expected %q got %q", i, d.expected, output)
-               }
-       }
-}
-
-func BenchmarkStripHTML(b *testing.B) {
-       b.ResetTimer()
-       for i := 0; i < b.N; i++ {
-               StripHTML(tstHTMLContent)
-       }
-}
-
 func TestStripEmptyNav(t *testing.T) {
        c := qt.New(t)
        cleaned := stripEmptyNav([]byte("do<nav>\n</nav>\n\nbedobedo"))

diff --git a/hugolib/page__per_output.go b/hugolib/page__per_output.go
index 6460b120b2307a291f78521f1396680c52262bc1..59299b709faec01c019ae12a023d9969919b8a31 100644 (file)
--- a/hugolib/page__per_output.go
+++ b/hugolib/page__per_output.go
@@ -201,7 +201,7 @@ func newPageContentOutput(p *pageState, po *pageOutput) (*pageContentOutput, err
        })
 
        cp.initPlain = cp.initMain.Branch(func() (any, error) {
-               cp.plain = helpers.StripHTML(string(cp.content))
+               cp.plain = tpl.StripHTML(string(cp.content))
                cp.plainWords = strings.Fields(cp.plain)
                cp.setWordCounts(p.m.isCJKLanguage)
 

diff --git a/hugolib/page_test.go b/hugolib/page_test.go
index b931731313c202e034953a68b45c2680272663c3..e5f8840a64ae66317c59c24709b1d284750f7962 100644 (file)
--- a/hugolib/page_test.go
+++ b/hugolib/page_test.go
@@ -26,6 +26,7 @@ import (
        "github.com/gohugoio/hugo/htesting"
        "github.com/gohugoio/hugo/markup/asciidocext"
        "github.com/gohugoio/hugo/markup/rst"
+       "github.com/gohugoio/hugo/tpl"
 
        "github.com/gohugoio/hugo/config"
 
@@ -40,7 +41,6 @@ import (
 
        qt "github.com/frankban/quicktest"
        "github.com/gohugoio/hugo/deps"
-       "github.com/gohugoio/hugo/helpers"
 )
 
 const (
@@ -351,7 +351,7 @@ func normalizeExpected(ext, str string) string {
        default:
                return str
        case "html":
-               return strings.Trim(helpers.StripHTML(str), " ")
+               return strings.Trim(tpl.StripHTML(str), " ")
        case "ad":
                paragraphs := strings.Split(str, "</p>")
                expected := ""
@@ -1736,6 +1736,7 @@ Len Summary: {{ len .Summary }}
 Len Content: {{ len .Content }}
 
 SUMMARY:{{ .Summary }}:{{ len .Summary }}:END
+
 `}
 
        b := newTestSitesBuilder(t)

diff --git a/tpl/internal/go_templates/htmltemplate/hugo_template.go b/tpl/internal/go_templates/htmltemplate/hugo_template.go
index eba54fbbf2528532aa62eb5752fddc8d7788473b..99edf8f68167477411740e5ff13f9ad14f1a1356 100644 (file)
--- a/tpl/internal/go_templates/htmltemplate/hugo_template.go
+++ b/tpl/internal/go_templates/htmltemplate/hugo_template.go
@@ -34,3 +34,8 @@ func (t *Template) Prepare() (*template.Template, error) {
        }
        return t.text, nil
 }
+
+// See https://github.com/golang/go/issues/5884
+func StripTags(html string) string {
+       return stripTags(html)
+}

diff --git a/tpl/strings/strings.go b/tpl/strings/strings.go
index 482a8a837e98ceb564e8a00f6d8bb799baa647d9..a49451483a8ea6e7d022bf68222c9a9ed737b368 100644 (file)
--- a/tpl/strings/strings.go
+++ b/tpl/strings/strings.go
@@ -25,6 +25,7 @@ import (
        "github.com/gohugoio/hugo/common/text"
        "github.com/gohugoio/hugo/deps"
        "github.com/gohugoio/hugo/helpers"
+       "github.com/gohugoio/hugo/tpl"
 
        "github.com/spf13/cast"
 )
@@ -52,7 +53,7 @@ func (ns *Namespace) CountRunes(s any) (int, error) {
        }
 
        counter := 0
-       for _, r := range helpers.StripHTML(ss) {
+       for _, r := range tpl.StripHTML(ss) {
                if !helpers.IsWhitespace(r) {
                        counter++
                }
@@ -83,11 +84,11 @@ func (ns *Namespace) CountWords(s any) (int, error) {
        }
 
        if !isCJKLanguage {
-               return len(strings.Fields(helpers.StripHTML((ss)))), nil
+               return len(strings.Fields(tpl.StripHTML(ss))), nil
        }
 
        counter := 0
-       for _, word := range strings.Fields(helpers.StripHTML(ss)) {
+       for _, word := range strings.Fields(tpl.StripHTML(ss)) {
                runeCount := utf8.RuneCountInString(word)
                if len(word) == runeCount {
                        counter++

diff --git a/tpl/template.go b/tpl/template.go
index 299b7208d0ec199c31a535eb45acfa96c87bdc38..738750de70c35ac96c79201c1f7413b9947e1c1d 100644 (file)
--- a/tpl/template.go
+++ b/tpl/template.go
@@ -18,9 +18,14 @@ import (
        "io"
        "reflect"
        "regexp"
+       "strings"
+       "unicode"
+
+       bp "github.com/gohugoio/hugo/bufferpool"
 
        "github.com/gohugoio/hugo/output"
 
+       htmltemplate "github.com/gohugoio/hugo/tpl/internal/go_templates/htmltemplate"
        texttemplate "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate"
 )
 
@@ -163,3 +168,44 @@ func GetHasLockFromContext(ctx context.Context) bool {
 func SetHasLockInContext(ctx context.Context, hasLock bool) context.Context {
        return context.WithValue(ctx, texttemplate.HasLockContextKey, hasLock)
 }
+
+const hugoNewLinePlaceholder = "___hugonl_"
+
+var (
+       stripHTMLReplacerPre = strings.NewReplacer("\n", " ", "</p>", hugoNewLinePlaceholder, "<br>", hugoNewLinePlaceholder, "<br />", hugoNewLinePlaceholder)
+       whitespaceRe         = regexp.MustCompile(`\s+`)
+)
+
+// StripHTML strips out all HTML tags in s.
+func StripHTML(s string) string {
+       // Shortcut strings with no tags in them
+       if !strings.ContainsAny(s, "<>") {
+               return s
+       }
+
+       pre := stripHTMLReplacerPre.Replace(s)
+       preReplaced := pre != s
+
+       s = htmltemplate.StripTags(pre)
+
+       if preReplaced {
+               s = strings.ReplaceAll(s, hugoNewLinePlaceholder, "\n")
+       }
+
+       var wasSpace bool
+       b := bp.GetBuffer()
+       defer bp.PutBuffer(b)
+       for _, r := range s {
+               isSpace := unicode.IsSpace(r)
+               if !(isSpace && wasSpace) {
+                       b.WriteRune(r)
+               }
+               wasSpace = isSpace
+       }
+
+       if b.Len() > 0 {
+               s = b.String()
+       }
+
+       return s
+}

diff --git a/tpl/template_test.go b/tpl/template_test.go
index afd3c4b0016bfd07401bd174e3a6c0346fc7e46f..d989b7158df892405e459453722983756a7cf6e2 100644 (file)
--- a/tpl/template_test.go
+++ b/tpl/template_test.go
@@ -28,3 +28,44 @@ func TestExtractBaseof(t *testing.T) {
        c.Assert(extractBaseOf("not baseof for you"), qt.Equals, "")
        c.Assert(extractBaseOf("template: blog/baseof.html:23:11:"), qt.Equals, "blog/baseof.html")
 }
+
+func TestStripHTML(t *testing.T) {
+       type test struct {
+               input, expected string
+       }
+       data := []test{
+               {"<h1>strip h1 tag <h1>", "strip h1 tag "},
+               {"<p> strip p tag </p>", " strip p tag "},
+               {"</br> strip br<br>", " strip br\n"},
+               {"</br> strip br2<br />", " strip br2\n"},
+               {"This <strong>is</strong> a\nnewline", "This is a newline"},
+               {"No Tags", "No Tags"},
+               {`<p>Summary Next Line.
+<figure >
+
+        <img src="/not/real" />
+
+
+</figure>
+.
+More text here.</p>
+
+<p>Some more text</p>`, "Summary Next Line. . More text here.\nSome more text\n"},
+
+               // Issue 9199
+               {"<div data-action='click->my-controller#doThing'>qwe</div>", "qwe"},
+               {"Hello, World!", "Hello, World!"},
+               {"foo&amp;bar", "foo&amp;bar"},
+               {`Hello <a href="www.example.com/">World</a>!`, "Hello World!"},
+               {"Foo <textarea>Bar</textarea> Baz", "Foo Bar Baz"},
+               {"Foo <!-- Bar --> Baz", "Foo Baz"},
+       }
+       for i, d := range data {
+               output := StripHTML(d.input)
+               if d.expected != output {
+                       t.Errorf("Test %d failed. Expected %q got %q", i, d.expected, output)
+               }
+       }
+}
+
+const tstHTMLContent = "<!DOCTYPE html><html><head><script src=\"http://two/foobar.js\"></script></head><body><nav><ul><li hugo-nav=\"section_0\"></li><li hugo-nav=\"section_1\"></li></ul></nav><article>content <a href=\"http://two/foobar\">foobar</a>. Follow up</article><p>This is some text.<br>And some more.</p></body></html>"

diff --git a/tpl/transform/transform.go b/tpl/transform/transform.go
index 7c091437847258c57d3a5defb9532b789a9c6797..69ab98a9a7a09edffdc205bb65855c7a0aa4d477 100644 (file)
--- a/tpl/transform/transform.go
+++ b/tpl/transform/transform.go
@@ -22,6 +22,7 @@ import (
        "github.com/gohugoio/hugo/cache/namedmemcache"
        "github.com/gohugoio/hugo/markup/converter/hooks"
        "github.com/gohugoio/hugo/markup/highlight"
+       "github.com/gohugoio/hugo/tpl"
 
        "github.com/gohugoio/hugo/deps"
        "github.com/gohugoio/hugo/helpers"
@@ -141,7 +142,7 @@ func (ns *Namespace) Plainify(s any) (string, error) {
                return "", err
        }
 
-       return helpers.StripHTML(ss), nil
+       return tpl.StripHTML(ss), nil
 }
 
 // For internal use.

diff --git a/tpl/transform/transform_test.go b/tpl/transform/transform_test.go
index ab2fd3b9ecc06de6aa442f8252a0c597f3471442..edef4e1bde504d3d66ff7df042b7ab05780ed06f 100644 (file)
--- a/tpl/transform/transform_test.go
+++ b/tpl/transform/transform_test.go
@@ -237,6 +237,7 @@ func TestPlainify(t *testing.T) {
                expect any
        }{
                {"<em>Note:</em> blah <b>blah</b>", "Note: blah blah"},
+               {"<div data-action='click->my-controller#doThing'>qwe</div>", "qwe"},
                // errors
                {tstNoStringer{}, false},
        } {