From 5136c879c29fdd713f664c68e6d61d4197b6fec0 Mon Sep 17 00:00:00 2001
From: Giteabot <teabot@gitea.io>
Date: Tue, 4 Jun 2024 21:26:55 +0800
Subject: [PATCH] Make pasted "img" tag has the same behavior as markdown image
(#31235) (#31243)
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 | 60 ++++++++++++++++++++--------
modules/markup/html_internal_test.go | 19 +++++----
modules/markup/html_test.go | 53 ++++++++++--------------
modules/markup/renderer.go | 2 +-
web_src/js/features/comp/Paste.js | 6 ++-
5 files changed, 80 insertions(+), 60 deletions(-)
diff --git a/modules/markup/html.go b/modules/markup/html.go
index 2958dc9646..44f88bbd11 100644
--- a/modules/markup/html.go
+++ b/modules/markup/html.go
@@ -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
diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go
index 3ff0597851..9aa9c22d70 100644
--- a/modules/markup/html_internal_test.go
+++ b/modules/markup/html_internal_test.go
@@ -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&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&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",
diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go
index 6b3bcd7257..a0642bcfa4 100644
--- a/modules/markup/html_test.go
+++ b/modules/markup/html_test.go
@@ -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"))
}
diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go
index dbd96a1e69..f372dcd5b7 100644
--- a/modules/markup/renderer.go
+++ b/modules/markup/renderer.go
@@ -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
diff --git a/web_src/js/features/comp/Paste.js b/web_src/js/features/comp/Paste.js
index b26296d1fc..35a7ceaef8 100644
--- a/web_src/js/features/comp/Paste.js
+++ b/web_src/js/features/comp/Paste.js
@@ -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 = ``;
}
editor.replacePlaceholder(placeholder, text);