Skip to content

Commit 5136c87

Browse files
GiteaBotwxiaoguanglunny
authored
Make pasted "img" tag has the same behavior as markdown image (#31235) (#31243)
Backport #31235 by wxiaoguang Fix #31230 --------- Co-authored-by: wxiaoguang <[email protected]> Co-authored-by: Lunny Xiao <[email protected]>
1 parent ca414a7 commit 5136c87

File tree

5 files changed

+80
-60
lines changed

5 files changed

+80
-60
lines changed

‎modules/markup/html.go

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,42 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
372372
return nil
373373
}
374374

375-
func visitNode(ctx *RenderContext, procs []processor, node *html.Node) {
375+
func handleNodeImg(ctx *RenderContext, img *html.Node) {
376+
for i, attr := range img.Attr {
377+
if attr.Key != "src" {
378+
continue
379+
}
380+
381+
if attr.Val != "" && !IsFullURLString(attr.Val) && !strings.HasPrefix(attr.Val, "/") {
382+
attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
383+
384+
// By default, the "<img>" tag should also be clickable,
385+
// because frontend use `<img>` to paste the re-scaled image into the markdown,
386+
// so it must match the default markdown image behavior.
387+
hasParentAnchor := false
388+
for p := img.Parent; p != nil; p = p.Parent {
389+
if hasParentAnchor = p.Type == html.ElementNode && p.Data == "a"; hasParentAnchor {
390+
break
391+
}
392+
}
393+
if !hasParentAnchor {
394+
imgA := &html.Node{Type: html.ElementNode, Data: "a", Attr: []html.Attribute{
395+
{Key: "href", Val: attr.Val},
396+
{Key: "target", Val: "_blank"},
397+
}}
398+
parent := img.Parent
399+
imgNext := img.NextSibling
400+
parent.RemoveChild(img)
401+
parent.InsertBefore(imgA, imgNext)
402+
imgA.AppendChild(img)
403+
}
404+
}
405+
attr.Val = camoHandleLink(attr.Val)
406+
img.Attr[i] = attr
407+
}
408+
}
409+
410+
func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Node {
376411
// Add user-content- to IDs and "#" links if they don't already have them
377412
for idx, attr := range node.Attr {
378413
val := strings.TrimPrefix(attr.Val, "#")
@@ -397,21 +432,14 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) {
397432
textNode(ctx, procs, node)
398433
case html.ElementNode:
399434
if node.Data == "img" {
400-
for i, attr := range node.Attr {
401-
if attr.Key != "src" {
402-
continue
403-
}
404-
if len(attr.Val) > 0 && !IsFullURLString(attr.Val) && !strings.HasPrefix(attr.Val, "data:image/") {
405-
attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
406-
}
407-
attr.Val = camoHandleLink(attr.Val)
408-
node.Attr[i] = attr
409-
}
435+
next := node.NextSibling
436+
handleNodeImg(ctx, node)
437+
return next
410438
} else if node.Data == "a" {
411439
// Restrict text in links to emojis
412440
procs = emojiProcessors
413441
} else if node.Data == "code" || node.Data == "pre" {
414-
return
442+
return node.NextSibling
415443
} else if node.Data == "i" {
416444
for _, attr := range node.Attr {
417445
if attr.Key != "class" {
@@ -434,11 +462,11 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) {
434462
}
435463
}
436464
}
437-
for n := node.FirstChild; n != nil; n = n.NextSibling {
438-
visitNode(ctx, procs, n)
465+
for n := node.FirstChild; n != nil; {
466+
n = visitNode(ctx, procs, n)
439467
}
440468
}
441-
// ignore everything else
469+
return node.NextSibling
442470
}
443471

444472
// textNode runs the passed node through various processors, in order to handle
@@ -851,7 +879,7 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
851879

852880
// FIXME: the use of "mode" is quite dirty and hacky, for example: what is a "document"? how should it be rendered?
853881
// The "mode" approach should be refactored to some other more clear&reliable way.
854-
crossLinkOnly := (ctx.Metas["mode"] == "document" && !ctx.IsWiki)
882+
crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki
855883

856884
var (
857885
found bool

‎modules/markup/html_internal_test.go

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ import (
1818

1919
const (
2020
TestAppURL = "http://localhost:3000/"
21-
TestOrgRepo = "gogits/gogs"
22-
TestRepoURL = TestAppURL + TestOrgRepo + "/"
21+
TestRepoURL = TestAppURL + "test-owner/test-repo/"
2322
)
2423

2524
// externalIssueLink an HTML link to an alphanumeric-style issue
@@ -64,8 +63,8 @@ var regexpMetas = map[string]string{
6463

6564
// these values should match the TestOrgRepo const above
6665
var localMetas = map[string]string{
67-
"user": "gogits",
68-
"repo": "gogs",
66+
"user": "test-owner",
67+
"repo": "test-repo",
6968
}
7069

7170
func TestRender_IssueIndexPattern(t *testing.T) {
@@ -362,12 +361,12 @@ func TestRender_FullIssueURLs(t *testing.T) {
362361
`Look here <a href="http://localhost:3000/person/repo/issues/4" class="ref-issue">person/repo#4</a>`)
363362
test("http://localhost:3000/person/repo/issues/4#issuecomment-1234",
364363
`<a href="http://localhost:3000/person/repo/issues/4#issuecomment-1234" class="ref-issue">person/repo#4 (comment)</a>`)
365-
test("http://localhost:3000/gogits/gogs/issues/4",
366-
`<a href="http://localhost:3000/gogits/gogs/issues/4" class="ref-issue">#4</a>`)
367-
test("http://localhost:3000/gogits/gogs/issues/4 test",
368-
`<a href="http://localhost:3000/gogits/gogs/issues/4" class="ref-issue">#4</a> test`)
369-
test("http://localhost:3000/gogits/gogs/issues/4?a=1&b=2#comment-123 test",
370-
`<a href="http://localhost:3000/gogits/gogs/issues/4?a=1&amp;b=2#comment-123" class="ref-issue">#4 (comment)</a> test`)
364+
test("http://localhost:3000/test-owner/test-repo/issues/4",
365+
`<a href="http://localhost:3000/test-owner/test-repo/issues/4" class="ref-issue">#4</a>`)
366+
test("http://localhost:3000/test-owner/test-repo/issues/4 test",
367+
`<a href="http://localhost:3000/test-owner/test-repo/issues/4" class="ref-issue">#4</a> test`)
368+
test("http://localhost:3000/test-owner/test-repo/issues/4?a=1&b=2#comment-123 test",
369+
`<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`)
371370
test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24",
372371
"http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24")
373372
test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files",

‎modules/markup/html_test.go

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func TestRender_Commits(t *testing.T) {
5353
}
5454

5555
sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d"
56-
repo := markup.TestRepoURL
56+
repo := "http://localhost:3000/gogits/gogs"
5757
commit := util.URLJoin(repo, "commit", sha)
5858
tree := util.URLJoin(repo, "tree", sha, "src")
5959

@@ -107,8 +107,8 @@ func TestRender_CrossReferences(t *testing.T) {
107107
}
108108

109109
test(
110-
"gogits/gogs#12345",
111-
`<p><a href="`+util.URLJoin(markup.TestAppURL, "gogits", "gogs", "issues", "12345")+`" class="ref-issue" rel="nofollow">gogits/gogs#12345</a></p>`)
110+
"test-owner/test-repo#12345",
111+
`<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>`)
112112
test(
113113
"go-gitea/gitea#12345",
114114
`<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) {
517517
}
518518

519519
func TestRender_RelativeImages(t *testing.T) {
520-
setting.AppURL = markup.TestAppURL
521-
522-
test := func(input, expected, expectedWiki string) {
520+
render := func(input string, isWiki bool, links markup.Links) string {
523521
buffer, err := markdown.RenderString(&markup.RenderContext{
524-
Ctx: git.DefaultContext,
525-
Links: markup.Links{
526-
Base: markup.TestRepoURL,
527-
BranchPath: "master",
528-
},
529-
Metas: localMetas,
530-
}, input)
531-
assert.NoError(t, err)
532-
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
533-
buffer, err = markdown.RenderString(&markup.RenderContext{
534-
Ctx: git.DefaultContext,
535-
Links: markup.Links{
536-
Base: markup.TestRepoURL,
537-
},
522+
Ctx: git.DefaultContext,
523+
Links: links,
538524
Metas: localMetas,
539-
IsWiki: true,
525+
IsWiki: isWiki,
540526
}, input)
541527
assert.NoError(t, err)
542-
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
528+
return strings.TrimSpace(string(buffer))
543529
}
544530

545-
rawwiki := util.URLJoin(markup.TestRepoURL, "wiki", "raw")
546-
mediatree := util.URLJoin(markup.TestRepoURL, "media", "master")
531+
out := render(`<img src="LINK">`, false, markup.Links{Base: "/test-owner/test-repo"})
532+
assert.Equal(t, `<a href="/test-owner/test-repo/LINK" target="_blank" rel="nofollow noopener"><img src="/test-owner/test-repo/LINK"/></a>`, out)
547533

548-
test(
549-
`<img src="Link">`,
550-
`<img src="`+util.URLJoin(mediatree, "Link")+`"/>`,
551-
`<img src="`+util.URLJoin(rawwiki, "Link")+`"/>`)
534+
out = render(`<img src="LINK">`, true, markup.Links{Base: "/test-owner/test-repo"})
535+
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)
552536

553-
test(
554-
`<img src="./icon.png">`,
555-
`<img src="`+util.URLJoin(mediatree, "icon.png")+`"/>`,
556-
`<img src="`+util.URLJoin(rawwiki, "icon.png")+`"/>`)
537+
out = render(`<img src="LINK">`, false, markup.Links{Base: "/test-owner/test-repo", BranchPath: "test-branch"})
538+
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)
539+
540+
out = render(`<img src="LINK">`, true, markup.Links{Base: "/test-owner/test-repo", BranchPath: "test-branch"})
541+
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)
542+
543+
out = render(`<img src="/LINK">`, true, markup.Links{Base: "/test-owner/test-repo", BranchPath: "test-branch"})
544+
assert.Equal(t, `<img src="/LINK"/>`, out)
557545
}
558546

559547
func Test_ParseClusterFuzz(t *testing.T) {
@@ -706,5 +694,6 @@ func TestIssue18471(t *testing.T) {
706694
func TestIsFullURL(t *testing.T) {
707695
assert.True(t, markup.IsFullURLString("https://example.com"))
708696
assert.True(t, markup.IsFullURLString("mailto:[email protected]"))
697+
assert.True(t, markup.IsFullURLString("data:image/11111"))
709698
assert.False(t, markup.IsFullURLString("/foo:bar"))
710699
}

‎modules/markup/renderer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ type RenderContext struct {
7373
Type string
7474
IsWiki bool
7575
Links Links
76-
Metas map[string]string
76+
Metas map[string]string // user, repo, mode(comment/document)
7777
DefaultLink string
7878
GitRepo *git.Repository
7979
ShaExistCache map[string]bool

‎web_src/js/features/comp/Paste.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,17 @@ async function handleClipboardImages(editor, dropzone, images, e) {
100100
const {uuid} = await uploadFile(img, uploadUrl);
101101
const {width, dppx} = await imageInfo(img);
102102

103-
const url = `/attachments/${uuid}`;
104103
let text;
105104
if (width > 0 && dppx > 1) {
106105
// Scale down images from HiDPI monitors. This uses the <img> tag because it's the only
107106
// method to change image size in Markdown that is supported by all implementations.
107+
// Make the image link relative to the repo path, then the final URL is "/sub-path/owner/repo/attachments/{uuid}"
108+
const url = `attachments/${uuid}`;
108109
text = `<img width="${Math.round(width / dppx)}" alt="${htmlEscape(name)}" src="${htmlEscape(url)}">`;
109110
} else {
111+
// Markdown always renders the image with a relative path, so the final URL is "/sub-path/owner/repo/attachments/{uuid}"
112+
// TODO: it should also use relative path for consistency, because absolute is ambiguous for "/sub-path/attachments" or "/attachments"
113+
const url = `/attachments/${uuid}`;
110114
text = `![${name}](${url})`;
111115
}
112116
editor.replacePlaceholder(placeholder, text);

0 commit comments

Comments
 (0)