Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make pasted "img" tag has the same behavior as markdown image #31235

Merged
merged 6 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions modules/markup/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "#")
Expand All @@ -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" {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 9 additions & 10 deletions modules/markup/html_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down
51 changes: 20 additions & 31 deletions modules/markup/html_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,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>`)
Expand Down Expand Up @@ -530,43 +530,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) {
Expand Down Expand Up @@ -719,5 +707,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"))
}
2 changes: 1 addition & 1 deletion modules/markup/renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,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
Repo gitrepo.Repository
Expand Down
6 changes: 5 additions & 1 deletion web_src/js/features/comp/Paste.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down