Skip to content

Commit b947bc4

Browse files
GiteaBotcharles7668wxiaoguang
authored
Fix footnote jump behavior on the issue page. (#34621) (#34669)
Backport #34621 by @charles7668 Close #34511 Close #34590 Add comment ID to the footnote item's id attribute to ensure uniqueness. Co-authored-by: charles <30816317+charles7668@users.noreply.github.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent 18dc41d commit b947bc4

File tree

13 files changed

+92
-24
lines changed

13 files changed

+92
-24
lines changed

‎models/issues/comment_code.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package issues
55

66
import (
77
"context"
8+
"strconv"
89

910
"code.gitea.io/gitea/models/db"
1011
"code.gitea.io/gitea/models/renderhelper"
@@ -114,7 +115,9 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu
114115
}
115116

116117
var err error
117-
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo)
118+
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{
119+
FootnoteContextID: strconv.FormatInt(comment.ID, 10),
120+
})
118121
if comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content); err != nil {
119122
return nil, err
120123
}

‎models/renderhelper/repo_comment.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ type RepoCommentOptions struct {
4444
DeprecatedRepoName string // it is only a patch for the non-standard "markup" api
4545
DeprecatedOwnerName string // it is only a patch for the non-standard "markup" api
4646
CurrentRefPath string // eg: "branch/main" or "commit/11223344"
47+
FootnoteContextID string // the extra context ID for footnotes, used to avoid conflicts with other footnotes in the same page
4748
}
4849

4950
func NewRenderContextRepoComment(ctx context.Context, repo *repo_model.Repository, opts ...RepoCommentOptions) *markup.RenderContext {
@@ -53,10 +54,11 @@ func NewRenderContextRepoComment(ctx context.Context, repo *repo_model.Repositor
5354
}
5455
rctx := markup.NewRenderContext(ctx)
5556
helper.ctx = rctx
57+
var metas map[string]string
5658
if repo != nil {
5759
helper.repoLink = repo.Link()
5860
helper.commitChecker = newCommitChecker(ctx, repo)
59-
rctx = rctx.WithMetas(repo.ComposeCommentMetas(ctx))
61+
metas = repo.ComposeCommentMetas(ctx)
6062
} else {
6163
// this is almost dead code, only to pass the incorrect tests
6264
helper.repoLink = fmt.Sprintf("%s/%s", helper.opts.DeprecatedOwnerName, helper.opts.DeprecatedRepoName)
@@ -68,6 +70,7 @@ func NewRenderContextRepoComment(ctx context.Context, repo *repo_model.Repositor
6870
"markupAllowShortIssuePattern": "true",
6971
})
7072
}
71-
rctx = rctx.WithHelper(helper)
73+
metas["footnoteContextId"] = helper.opts.FootnoteContextID
74+
rctx = rctx.WithMetas(metas).WithHelper(helper)
7275
return rctx
7376
}

‎modules/markup/common/footnote.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,9 @@ func (r *FootnoteHTMLRenderer) renderFootnoteLink(w util.BufWriter, source []byt
409409
_, _ = w.Write(n.Name)
410410
_, _ = w.WriteString(`"><a href="#fn:`)
411411
_, _ = w.Write(n.Name)
412-
_, _ = w.WriteString(`" class="footnote-ref" role="doc-noteref">`)
412+
_, _ = w.WriteString(`" class="footnote-ref" role="doc-noteref">`) // FIXME: here and below, need to keep the classes
413413
_, _ = w.WriteString(is)
414-
_, _ = w.WriteString(`</a></sup>`)
414+
_, _ = w.WriteString(` </a></sup>`) // the style doesn't work at the moment, so add a space to separate the names
415415
}
416416
return ast.WalkContinue, nil
417417
}

‎modules/markup/html.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Nod
320320
}
321321

322322
processNodeAttrID(node)
323+
processFootnoteNode(ctx, node) // FIXME: the footnote processing should be done in the "footnote.go" renderer directly
323324

324325
if isEmojiNode(node) {
325326
// TextNode emoji will be converted to `<span class="emoji">`, then the next iteration will visit the "span"

‎modules/markup/html_issue_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func TestRender_IssueList(t *testing.T) {
3030
rctx := markup.NewTestRenderContext(markup.TestAppURL, map[string]string{
3131
"user": "test-user", "repo": "test-repo",
3232
"markupAllowShortIssuePattern": "true",
33+
"footnoteContextId": "12345",
3334
})
3435
out, err := markdown.RenderString(rctx, input)
3536
require.NoError(t, err)
@@ -69,4 +70,22 @@ func TestRender_IssueList(t *testing.T) {
6970
</ul>`,
7071
)
7172
})
73+
74+
t.Run("IssueFootnote", func(t *testing.T) {
75+
test(
76+
"foo[^1][^2]\n\n[^1]: bar\n[^2]: baz",
77+
`<p>foo<sup id="fnref:user-content-1-12345"><a href="#fn:user-content-1-12345" rel="nofollow">1 </a></sup><sup id="fnref:user-content-2-12345"><a href="#fn:user-content-2-12345" rel="nofollow">2 </a></sup></p>
78+
<div>
79+
<hr/>
80+
<ol>
81+
<li id="fn:user-content-1-12345">
82+
<p>bar <a href="#fnref:user-content-1-12345" rel="nofollow">↩︎</a></p>
83+
</li>
84+
<li id="fn:user-content-2-12345">
85+
<p>baz <a href="#fnref:user-content-2-12345" rel="nofollow">↩︎</a></p>
86+
</li>
87+
</ol>
88+
</div>`,
89+
)
90+
})
7291
}

‎modules/markup/html_node.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ func isAnchorIDUserContent(s string) bool {
1515
return strings.HasPrefix(s, "user-content-") || strings.Contains(s, ":user-content-")
1616
}
1717

18+
func isAnchorIDFootnote(s string) bool {
19+
return strings.HasPrefix(s, "fnref:user-content-") || strings.HasPrefix(s, "fn:user-content-")
20+
}
21+
22+
func isAnchorHrefFootnote(s string) bool {
23+
return strings.HasPrefix(s, "#fnref:user-content-") || strings.HasPrefix(s, "#fn:user-content-")
24+
}
25+
1826
func processNodeAttrID(node *html.Node) {
1927
// Add user-content- to IDs and "#" links if they don't already have them,
2028
// and convert the link href to a relative link to the host root
@@ -27,6 +35,18 @@ func processNodeAttrID(node *html.Node) {
2735
}
2836
}
2937

38+
func processFootnoteNode(ctx *RenderContext, node *html.Node) {
39+
for idx, attr := range node.Attr {
40+
if (attr.Key == "id" && isAnchorIDFootnote(attr.Val)) ||
41+
(attr.Key == "href" && isAnchorHrefFootnote(attr.Val)) {
42+
if footnoteContextID := ctx.RenderOptions.Metas["footnoteContextId"]; footnoteContextID != "" {
43+
node.Attr[idx].Val = attr.Val + "-" + footnoteContextID
44+
}
45+
continue
46+
}
47+
}
48+
}
49+
3050
func processNodeA(ctx *RenderContext, node *html.Node) {
3151
for idx, attr := range node.Attr {
3252
if attr.Key == "href" {

‎modules/markup/markdown/markdown_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ This PR has been generated by [Renovate Bot](https://github.com/renovatebot/reno
223223
<dd>This is another definition of the second term.</dd>
224224
</dl>
225225
<h3 id="user-content-footnotes">Footnotes</h3>
226-
<p>Here is a simple footnote,<sup id="fnref:user-content-1"><a href="#fn:user-content-1" rel="nofollow">1</a></sup> and here is a longer one.<sup id="fnref:user-content-bignote"><a href="#fn:user-content-bignote" rel="nofollow">2</a></sup></p>
226+
<p>Here is a simple footnote,<sup id="fnref:user-content-1"><a href="#fn:user-content-1" rel="nofollow">1 </a></sup> and here is a longer one.<sup id="fnref:user-content-bignote"><a href="#fn:user-content-bignote" rel="nofollow">2 </a></sup></p>
227227
<div>
228228
<hr/>
229229
<ol>

‎routers/common/markup.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,11 @@ func RenderMarkup(ctx *context.Base, ctxRepo *context.Repository, mode, text, ur
7676
})
7777
rctx = rctx.WithMarkupType(markdown.MarkupName)
7878
case "comment":
79-
rctx = renderhelper.NewRenderContextRepoComment(ctx, repoModel, renderhelper.RepoCommentOptions{DeprecatedOwnerName: repoOwnerName, DeprecatedRepoName: repoName})
79+
rctx = renderhelper.NewRenderContextRepoComment(ctx, repoModel, renderhelper.RepoCommentOptions{
80+
DeprecatedOwnerName: repoOwnerName,
81+
DeprecatedRepoName: repoName,
82+
FootnoteContextID: "preview",
83+
})
8084
rctx = rctx.WithMarkupType(markdown.MarkupName)
8185
case "wiki":
8286
rctx = renderhelper.NewRenderContextRepoWiki(ctx, repoModel, renderhelper.RepoWikiOptions{DeprecatedOwnerName: repoOwnerName, DeprecatedRepoName: repoName})

‎routers/web/repo/issue.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,9 @@ func UpdateIssueContent(ctx *context.Context) {
364364
}
365365
}
366366

367-
rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository)
367+
rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{
368+
FootnoteContextID: "0",
369+
})
368370
content, err := markdown.RenderString(rctx, issue.Content)
369371
if err != nil {
370372
ctx.ServerError("RenderString", err)

‎routers/web/repo/issue_comment.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"html/template"
1010
"net/http"
11+
"strconv"
1112

1213
issues_model "code.gitea.io/gitea/models/issues"
1314
"code.gitea.io/gitea/models/renderhelper"
@@ -278,7 +279,9 @@ func UpdateCommentContent(ctx *context.Context) {
278279

279280
var renderedContent template.HTML
280281
if comment.Content != "" {
281-
rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository)
282+
rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{
283+
FootnoteContextID: strconv.FormatInt(comment.ID, 10),
284+
})
282285
renderedContent, err = markdown.RenderString(rctx, comment.Content)
283286
if err != nil {
284287
ctx.ServerError("RenderString", err)

‎routers/web/repo/issue_view.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/http"
1010
"net/url"
1111
"sort"
12+
"strconv"
1213

1314
activities_model "code.gitea.io/gitea/models/activities"
1415
"code.gitea.io/gitea/models/db"
@@ -624,7 +625,9 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
624625
comment.Issue = issue
625626

626627
if comment.Type == issues_model.CommentTypeComment || comment.Type == issues_model.CommentTypeReview {
627-
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo)
628+
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{
629+
FootnoteContextID: strconv.FormatInt(comment.ID, 10),
630+
})
628631
comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content)
629632
if err != nil {
630633
ctx.ServerError("RenderString", err)
@@ -700,7 +703,9 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue
700703
}
701704
}
702705
} else if comment.Type.HasContentSupport() {
703-
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo)
706+
rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo, renderhelper.RepoCommentOptions{
707+
FootnoteContextID: strconv.FormatInt(comment.ID, 10),
708+
})
704709
comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content)
705710
if err != nil {
706711
ctx.ServerError("RenderString", err)
@@ -981,7 +986,9 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss
981986

982987
func prepareIssueViewContent(ctx *context.Context, issue *issues_model.Issue) {
983988
var err error
984-
rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository)
989+
rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{
990+
FootnoteContextID: "0", // Set footnote context ID to 0 for the issue content
991+
})
985992
issue.RenderedContent, err = markdown.RenderString(rctx, issue.Content)
986993
if err != nil {
987994
ctx.ServerError("RenderString", err)

‎routers/web/repo/release.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"errors"
99
"fmt"
1010
"net/http"
11+
"strconv"
1112
"strings"
1213

1314
"code.gitea.io/gitea/models/db"
@@ -113,7 +114,9 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions)
113114
cacheUsers[r.PublisherID] = r.Publisher
114115
}
115116

116-
rctx := renderhelper.NewRenderContextRepoComment(ctx, r.Repo)
117+
rctx := renderhelper.NewRenderContextRepoComment(ctx, r.Repo, renderhelper.RepoCommentOptions{
118+
FootnoteContextID: strconv.FormatInt(r.ID, 10),
119+
})
117120
r.RenderedNote, err = markdown.RenderString(rctx, r.Note)
118121
if err != nil {
119122
return nil, err

‎web_src/js/markup/anchors.ts

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,24 @@ const removePrefix = (str: string): string => str.replace(/^user-content-/, '');
55
const hasPrefix = (str: string): boolean => str.startsWith('user-content-');
66

77
// scroll to anchor while respecting the `user-content` prefix that exists on the target
8-
function scrollToAnchor(encodedId: string): void {
9-
if (!encodedId) return;
10-
const id = decodeURIComponent(encodedId);
11-
const prefixedId = addPrefix(id);
12-
let el = document.querySelector(`#${prefixedId}`);
8+
function scrollToAnchor(encodedId?: string): void {
9+
// FIXME: need to rewrite this function with new a better markup anchor generation logic, too many tricks here
10+
let elemId: string;
11+
try {
12+
elemId = decodeURIComponent(encodedId ?? '');
13+
} catch {} // ignore the errors, since the "encodedId" is from user's input
14+
if (!elemId) return;
15+
16+
const prefixedId = addPrefix(elemId);
17+
// eslint-disable-next-line unicorn/prefer-query-selector
18+
let el = document.getElementById(prefixedId);
1319

1420
// check for matching user-generated `a[name]`
15-
if (!el) {
16-
el = document.querySelector(`a[name="${CSS.escape(prefixedId)}"]`);
17-
}
21+
el = el ?? document.querySelector(`a[name="${CSS.escape(prefixedId)}"]`);
1822

1923
// compat for links with old 'user-content-' prefixed hashes
20-
if (!el && hasPrefix(id)) {
21-
return document.querySelector(`#${id}`)?.scrollIntoView();
22-
}
24+
// eslint-disable-next-line unicorn/prefer-query-selector
25+
el = (!el && hasPrefix(elemId)) ? document.getElementById(elemId) : el;
2326

2427
el?.scrollIntoView();
2528
}

0 commit comments

Comments
 (0)