Make sure sibling images get a link too (#14979)

* Make sure sibling images get a link too

Due a problem with the ast.Walker in the our transformer in goldmark
an image with a sibling image will not be transformed to gain a parent
link. This PR fixes this.

Fix #12925

Signed-off-by: Andrew Thornton <art27@cantab.net>

diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go
index f9fd6eb9764bba904592f8c15d44b9a4ff698123..148067f1b07d62ac9fc179a3aaa3b25064ff0a20 100644 (file)
--- a/modules/markup/markdown/goldmark.go
+++ b/modules/markup/markdown/goldmark.go
@@ -10,6 +10,7 @@ import (
        "regexp"
        "strings"
 
+       "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/markup"
        "code.gitea.io/gitea/modules/markup/common"
        "code.gitea.io/gitea/modules/setting"
@@ -101,11 +102,41 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
                        parent := n.Parent()
                        // Create a link around image only if parent is not already a link
                        if _, ok := parent.(*ast.Link); !ok && parent != nil {
+                               next := n.NextSibling()
+
+                               // Create a link wrapper
                                wrap := ast.NewLink()
                                wrap.Destination = link
                                wrap.Title = v.Title
+
+                               // Duplicate the current image node
+                               image := ast.NewImage(ast.NewLink())
+                               image.Destination = link
+                               image.Title = v.Title
+                               for _, attr := range v.Attributes() {
+                                       image.SetAttribute(attr.Name, attr.Value)
+                               }
+                               for child := v.FirstChild(); child != nil; {
+                                       next := child.NextSibling()
+                                       image.AppendChild(image, child)
+                                       child = next
+                               }
+
+                               // Append our duplicate image to the wrapper link
+                               wrap.AppendChild(wrap, image)
+
+                               // Wire in the next sibling
+                               wrap.SetNextSibling(next)
+
+                               // Replace the current node with the wrapper link
                                parent.ReplaceChild(parent, n, wrap)
-                               wrap.AppendChild(wrap, n)
+
+                               // But most importantly ensure the next sibling is still on the old image too
+                               v.SetNextSibling(next)
+
+                       } else {
+                               log.Debug("ast.Image: %s has parent: %v", link, parent)
+
                        }
                case *ast.Link:
                        // Links need their href to munged to be a real value

diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go
index 89005fc25d8e439a2aba10bc9645ea1f5bf40398..3196beea1f945d64f6a64a6e813bc3a249ac4168 100644 (file)
--- a/modules/markup/markdown/markdown_test.go
+++ b/modules/markup/markdown/markdown_test.go
@@ -308,3 +308,14 @@ func TestRender_RenderParagraphs(t *testing.T) {
        test(t, "A\n\nB\nC\n", 2)
        test(t, "A\n\n\nB\nC\n", 2)
 }
+
+func TestRenderSiblingImages_Issue12925(t *testing.T) {
+       testcase := `![image1](/image1)
+![image2](/image2)
+`
+       expected := `<p><a href="/image1" rel="nofollow"><img src="/image1" alt="image1"></a><br>
+<a href="/image2" rel="nofollow"><img src="/image2" alt="image2"></a></p>
+`
+       res := string(RenderRaw([]byte(testcase), "", false))
+       assert.Equal(t, expected, res)
+}