]> source.dussan.org Git - gitea.git/commitdiff
Make pasted "img" tag has the same behavior as markdown image (#31235) (#31243)
authorGiteabot <teabot@gitea.io>
Tue, 4 Jun 2024 13:26:55 +0000 (21:26 +0800)
committerGitHub <noreply@github.com>
Tue, 4 Jun 2024 13:26:55 +0000 (13:26 +0000)
Backport #31235 by wxiaoguang

Fix #31230

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
modules/markup/html.go
modules/markup/html_internal_test.go
modules/markup/html_test.go
modules/markup/renderer.go
web_src/js/features/comp/Paste.js

index 2958dc964671f37c168e41bb5657250f7d7e27b5..44f88bbd113657ddf1531fe4233853290aad682a 100644 (file)
@@ -372,7 +372,42 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
        return nil
 }
 
-func visitNode(ctx *RenderContext, procs []processor, node *html.Node) {
+func handleNodeImg(ctx *RenderContext, img *html.Node) {
+       for i, attr := range img.Attr {
+               if attr.Key != "src" {
+                       continue
+               }
+
+               if attr.Val != "" && !IsFullURLString(attr.Val) && !strings.HasPrefix(attr.Val, "/") {
+                       attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
+
+                       // By default, the "<img>" tag should also be clickable,
+                       // because frontend use `<img>` to paste the re-scaled image into the markdown,
+                       // so it must match the default markdown image behavior.
+                       hasParentAnchor := false
+                       for p := img.Parent; p != nil; p = p.Parent {
+                               if hasParentAnchor = p.Type == html.ElementNode && p.Data == "a"; hasParentAnchor {
+                                       break
+                               }
+                       }
+                       if !hasParentAnchor {
+                               imgA := &html.Node{Type: html.ElementNode, Data: "a", Attr: []html.Attribute{
+                                       {Key: "href", Val: attr.Val},
+                                       {Key: "target", Val: "_blank"},
+                               }}
+                               parent := img.Parent
+                               imgNext := img.NextSibling
+                               parent.RemoveChild(img)
+                               parent.InsertBefore(imgA, imgNext)
+                               imgA.AppendChild(img)
+                       }
+               }
+               attr.Val = camoHandleLink(attr.Val)
+               img.Attr[i] = attr
+       }
+}
+
+func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Node {
        // Add user-content- to IDs and "#" links if they don't already have them
        for idx, attr := range node.Attr {
                val := strings.TrimPrefix(attr.Val, "#")
@@ -397,21 +432,14 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) {
                textNode(ctx, procs, node)
        case html.ElementNode:
                if node.Data == "img" {
-                       for i, attr := range node.Attr {
-                               if attr.Key != "src" {
-                                       continue
-                               }
-                               if len(attr.Val) > 0 && !IsFullURLString(attr.Val) && !strings.HasPrefix(attr.Val, "data:image/") {
-                                       attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
-                               }
-                               attr.Val = camoHandleLink(attr.Val)
-                               node.Attr[i] = attr
-                       }
+                       next := node.NextSibling
+                       handleNodeImg(ctx, node)
+                       return next
                } else if node.Data == "a" {
                        // Restrict text in links to emojis
                        procs = emojiProcessors
                } else if node.Data == "code" || node.Data == "pre" {
-                       return
+                       return node.NextSibling
                } else if node.Data == "i" {
                        for _, attr := range node.Attr {
                                if attr.Key != "class" {
@@ -434,11 +462,11 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) {
                                }
                        }
                }
-               for n := node.FirstChild; n != nil; n = n.NextSibling {
-                       visitNode(ctx, procs, n)
+               for n := node.FirstChild; n != nil; {
+                       n = visitNode(ctx, procs, n)
                }
        }
-       // ignore everything else
+       return node.NextSibling
 }
 
 // textNode runs the passed node through various processors, in order to handle
@@ -851,7 +879,7 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 
        // FIXME: the use of "mode" is quite dirty and hacky, for example: what is a "document"? how should it be rendered?
        // The "mode" approach should be refactored to some other more clear&reliable way.
-       crossLinkOnly := (ctx.Metas["mode"] == "document" && !ctx.IsWiki)
+       crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki
 
        var (
                found bool
index 3ff0597851bba60030d63a8729c395bb992ec698..9aa9c22d701f4dc1570e44a67ab5a32e67748bb3 100644 (file)
@@ -18,8 +18,7 @@ import (
 
 const (
        TestAppURL  = "http://localhost:3000/"
-       TestOrgRepo = "gogits/gogs"
-       TestRepoURL = TestAppURL + TestOrgRepo + "/"
+       TestRepoURL = TestAppURL + "test-owner/test-repo/"
 )
 
 // externalIssueLink an HTML link to an alphanumeric-style issue
@@ -64,8 +63,8 @@ var regexpMetas = map[string]string{
 
 // these values should match the TestOrgRepo const above
 var localMetas = map[string]string{
-       "user": "gogits",
-       "repo": "gogs",
+       "user": "test-owner",
+       "repo": "test-repo",
 }
 
 func TestRender_IssueIndexPattern(t *testing.T) {
@@ -362,12 +361,12 @@ func TestRender_FullIssueURLs(t *testing.T) {
                `Look here <a href="http://localhost:3000/person/repo/issues/4" class="ref-issue">person/repo#4</a>`)
        test("http://localhost:3000/person/repo/issues/4#issuecomment-1234",
                `<a href="http://localhost:3000/person/repo/issues/4#issuecomment-1234" class="ref-issue">person/repo#4 (comment)</a>`)
-       test("http://localhost:3000/gogits/gogs/issues/4",
-               `<a href="http://localhost:3000/gogits/gogs/issues/4" class="ref-issue">#4</a>`)
-       test("http://localhost:3000/gogits/gogs/issues/4 test",
-               `<a href="http://localhost:3000/gogits/gogs/issues/4" class="ref-issue">#4</a> test`)
-       test("http://localhost:3000/gogits/gogs/issues/4?a=1&b=2#comment-123 test",
-               `<a href="http://localhost:3000/gogits/gogs/issues/4?a=1&amp;b=2#comment-123" class="ref-issue">#4 (comment)</a> test`)
+       test("http://localhost:3000/test-owner/test-repo/issues/4",
+               `<a href="http://localhost:3000/test-owner/test-repo/issues/4" class="ref-issue">#4</a>`)
+       test("http://localhost:3000/test-owner/test-repo/issues/4 test",
+               `<a href="http://localhost:3000/test-owner/test-repo/issues/4" class="ref-issue">#4</a> test`)
+       test("http://localhost:3000/test-owner/test-repo/issues/4?a=1&b=2#comment-123 test",
+               `<a href="http://localhost:3000/test-owner/test-repo/issues/4?a=1&amp;b=2#comment-123" class="ref-issue">#4 (comment)</a> test`)
        test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24",
                "http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24")
        test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files",
index 6b3bcd72578341abdb44bac1ca94fe5126aca9cd..a0642bcfa4127b64877da74171575043ca537d89 100644 (file)
@@ -53,7 +53,7 @@ func TestRender_Commits(t *testing.T) {
        }
 
        sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d"
-       repo := markup.TestRepoURL
+       repo := "http://localhost:3000/gogits/gogs"
        commit := util.URLJoin(repo, "commit", sha)
        tree := util.URLJoin(repo, "tree", sha, "src")
 
@@ -107,8 +107,8 @@ func TestRender_CrossReferences(t *testing.T) {
        }
 
        test(
-               "gogits/gogs#12345",
-               `<p><a href="`+util.URLJoin(markup.TestAppURL, "gogits", "gogs", "issues", "12345")+`" class="ref-issue" rel="nofollow">gogits/gogs#12345</a></p>`)
+               "test-owner/test-repo#12345",
+               `<p><a href="`+util.URLJoin(markup.TestAppURL, "test-owner", "test-repo", "issues", "12345")+`" class="ref-issue" rel="nofollow">test-owner/test-repo#12345</a></p>`)
        test(
                "go-gitea/gitea#12345",
                `<p><a href="`+util.URLJoin(markup.TestAppURL, "go-gitea", "gitea", "issues", "12345")+`" class="ref-issue" rel="nofollow">go-gitea/gitea#12345</a></p>`)
@@ -517,43 +517,31 @@ func TestRender_ShortLinks(t *testing.T) {
 }
 
 func TestRender_RelativeImages(t *testing.T) {
-       setting.AppURL = markup.TestAppURL
-
-       test := func(input, expected, expectedWiki string) {
+       render := func(input string, isWiki bool, links markup.Links) string {
                buffer, err := markdown.RenderString(&markup.RenderContext{
-                       Ctx: git.DefaultContext,
-                       Links: markup.Links{
-                               Base:       markup.TestRepoURL,
-                               BranchPath: "master",
-                       },
-                       Metas: localMetas,
-               }, input)
-               assert.NoError(t, err)
-               assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
-               buffer, err = markdown.RenderString(&markup.RenderContext{
-                       Ctx: git.DefaultContext,
-                       Links: markup.Links{
-                               Base: markup.TestRepoURL,
-                       },
+                       Ctx:    git.DefaultContext,
+                       Links:  links,
                        Metas:  localMetas,
-                       IsWiki: true,
+                       IsWiki: isWiki,
                }, input)
                assert.NoError(t, err)
-               assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
+               return strings.TrimSpace(string(buffer))
        }
 
-       rawwiki := util.URLJoin(markup.TestRepoURL, "wiki", "raw")
-       mediatree := util.URLJoin(markup.TestRepoURL, "media", "master")
+       out := render(`<img src="LINK">`, false, markup.Links{Base: "/test-owner/test-repo"})
+       assert.Equal(t, `<a href="/test-owner/test-repo/LINK" target="_blank" rel="nofollow noopener"><img src="/test-owner/test-repo/LINK"/></a>`, out)
 
-       test(
-               `<img src="Link">`,
-               `<img src="`+util.URLJoin(mediatree, "Link")+`"/>`,
-               `<img src="`+util.URLJoin(rawwiki, "Link")+`"/>`)
+       out = render(`<img src="LINK">`, true, markup.Links{Base: "/test-owner/test-repo"})
+       assert.Equal(t, `<a href="/test-owner/test-repo/wiki/raw/LINK" target="_blank" rel="nofollow noopener"><img src="/test-owner/test-repo/wiki/raw/LINK"/></a>`, out)
 
-       test(
-               `<img src="./icon.png">`,
-               `<img src="`+util.URLJoin(mediatree, "icon.png")+`"/>`,
-               `<img src="`+util.URLJoin(rawwiki, "icon.png")+`"/>`)
+       out = render(`<img src="LINK">`, false, markup.Links{Base: "/test-owner/test-repo", BranchPath: "test-branch"})
+       assert.Equal(t, `<a href="/test-owner/test-repo/media/test-branch/LINK" target="_blank" rel="nofollow noopener"><img src="/test-owner/test-repo/media/test-branch/LINK"/></a>`, out)
+
+       out = render(`<img src="LINK">`, true, markup.Links{Base: "/test-owner/test-repo", BranchPath: "test-branch"})
+       assert.Equal(t, `<a href="/test-owner/test-repo/wiki/raw/LINK" target="_blank" rel="nofollow noopener"><img src="/test-owner/test-repo/wiki/raw/LINK"/></a>`, out)
+
+       out = render(`<img src="/LINK">`, true, markup.Links{Base: "/test-owner/test-repo", BranchPath: "test-branch"})
+       assert.Equal(t, `<img src="/LINK"/>`, out)
 }
 
 func Test_ParseClusterFuzz(t *testing.T) {
@@ -706,5 +694,6 @@ func TestIssue18471(t *testing.T) {
 func TestIsFullURL(t *testing.T) {
        assert.True(t, markup.IsFullURLString("https://example.com"))
        assert.True(t, markup.IsFullURLString("mailto:test@example.com"))
+       assert.True(t, markup.IsFullURLString("data:image/11111"))
        assert.False(t, markup.IsFullURLString("/foo:bar"))
 }
index dbd96a1e69dc66ccc4ba876e8d1158e2b4b27a89..f372dcd5b7bffcac4084f5767aab80aa09bdae79 100644 (file)
@@ -73,7 +73,7 @@ type RenderContext struct {
        Type             string
        IsWiki           bool
        Links            Links
-       Metas            map[string]string
+       Metas            map[string]string // user, repo, mode(comment/document)
        DefaultLink      string
        GitRepo          *git.Repository
        ShaExistCache    map[string]bool
index b26296d1fc967efd0141b99894214f7abd7024a9..35a7ceaef8be26ab65a415dd5167761c796657b2 100644 (file)
@@ -100,13 +100,17 @@ async function handleClipboardImages(editor, dropzone, images, e) {
     const {uuid} = await uploadFile(img, uploadUrl);
     const {width, dppx} = await imageInfo(img);
 
-    const url = `/attachments/${uuid}`;
     let text;
     if (width > 0 && dppx > 1) {
       // Scale down images from HiDPI monitors. This uses the <img> tag because it's the only
       // method to change image size in Markdown that is supported by all implementations.
+      // Make the image link relative to the repo path, then the final URL is "/sub-path/owner/repo/attachments/{uuid}"
+      const url = `attachments/${uuid}`;
       text = `<img width="${Math.round(width / dppx)}" alt="${htmlEscape(name)}" src="${htmlEscape(url)}">`;
     } else {
+      // Markdown always renders the image with a relative path, so the final URL is "/sub-path/owner/repo/attachments/{uuid}"
+      // TODO: it should also use relative path for consistency, because absolute is ambiguous for "/sub-path/attachments" or "/attachments"
+      const url = `/attachments/${uuid}`;
       text = `![${name}](${url})`;
     }
     editor.replacePlaceholder(placeholder, text);