diff --git a/models/renderhelper/repo_file.go b/models/renderhelper/repo_file.go index e0375ed280..f1df8e89e0 100644 --- a/models/renderhelper/repo_file.go +++ b/models/renderhelper/repo_file.go @@ -70,6 +70,6 @@ func NewRenderContextRepoFile(ctx context.Context, repo *repo_model.Repository, "repo": helper.opts.DeprecatedRepoName, }) } - rctx = rctx.WithHelper(helper) + rctx = rctx.WithHelper(helper).WithEnableHeadingIDGeneration(true) return rctx } diff --git a/models/renderhelper/repo_wiki.go b/models/renderhelper/repo_wiki.go index b75f1b9701..218b1e4a67 100644 --- a/models/renderhelper/repo_wiki.go +++ b/models/renderhelper/repo_wiki.go @@ -71,7 +71,7 @@ func NewRenderContextRepoWiki(ctx context.Context, repo *repo_model.Repository, "markupAllowShortIssuePattern": "true", }) } - rctx = rctx.WithHelper(helper) + rctx = rctx.WithHelper(helper).WithEnableHeadingIDGeneration(true) helper.ctx = rctx return rctx } diff --git a/modules/markup/html.go b/modules/markup/html.go index 51afd4be00..9676241b54 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -314,7 +314,7 @@ func visitNode(ctx *RenderContext, procs []processor, node *html.Node) *html.Nod return node.NextSibling } - processNodeAttrID(node) + processNodeAttrID(ctx, node) processFootnoteNode(ctx, node) // FIXME: the footnote processing should be done in the "footnote.go" renderer directly if isEmojiNode(node) { diff --git a/modules/markup/html_node.go b/modules/markup/html_node.go index 4eb78fdd2b..066ee9711d 100644 --- a/modules/markup/html_node.go +++ b/modules/markup/html_node.go @@ -6,6 +6,8 @@ package markup import ( "strings" + "code.gitea.io/gitea/modules/markup/common" + "golang.org/x/net/html" ) @@ -23,16 +25,57 @@ func isAnchorHrefFootnote(s string) bool { return strings.HasPrefix(s, "#fnref:user-content-") || strings.HasPrefix(s, "#fn:user-content-") } -func processNodeAttrID(node *html.Node) { +// isHeadingTag returns true if the node is a heading tag (h1-h6) +func isHeadingTag(node *html.Node) bool { + return node.Type == html.ElementNode && + len(node.Data) == 2 && + node.Data[0] == 'h' && + node.Data[1] >= '1' && node.Data[1] <= '6' +} + +// getNodeText extracts the text content from a node and its children +func getNodeText(node *html.Node) string { + var text strings.Builder + var extractText func(*html.Node) + extractText = func(n *html.Node) { + if n.Type == html.TextNode { + text.WriteString(n.Data) + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + extractText(c) + } + } + extractText(node) + return text.String() +} + +func processNodeAttrID(ctx *RenderContext, node *html.Node) { // Add user-content- to IDs and "#" links if they don't already have them, // and convert the link href to a relative link to the host root + hasID := false for idx, attr := range node.Attr { if attr.Key == "id" { + hasID = true if !isAnchorIDUserContent(attr.Val) { node.Attr[idx].Val = "user-content-" + attr.Val } } } + + // For heading tags (h1-h6) without an id attribute, generate one from the text content. + // This ensures HTML headings like

Title

get proper permalink anchors + // matching the behavior of Markdown headings. + // Only enabled for repository files and wiki pages via EnableHeadingIDGeneration option. + if !hasID && isHeadingTag(node) && ctx.RenderOptions.EnableHeadingIDGeneration { + text := getNodeText(node) + if text != "" { + // Use the same CleanValue function used by Markdown heading ID generation + cleanedID := string(common.CleanValue([]byte(text))) + if cleanedID != "" { + node.Attr = append(node.Attr, html.Attribute{Key: "id", Val: "user-content-" + cleanedID}) + } + } + } } func processFootnoteNode(ctx *RenderContext, node *html.Node) { diff --git a/modules/markup/html_node_test.go b/modules/markup/html_node_test.go new file mode 100644 index 0000000000..007e3c2a12 --- /dev/null +++ b/modules/markup/html_node_test.go @@ -0,0 +1,104 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markup + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestProcessNodeAttrID_HTMLHeadingWithoutID(t *testing.T) { + // Test that HTML headings without id get an auto-generated id from their text content + // when EnableHeadingIDGeneration is true (for repo files and wiki pages) + testCases := []struct { + name string + input string + expected string + }{ + { + name: "h1 without id", + input: `

Heading without ID

`, + expected: `

Heading without ID

`, + }, + { + name: "h2 without id", + input: `

Another Heading

`, + expected: `

Another Heading

`, + }, + { + name: "h3 without id", + input: `

Third Level

`, + expected: `

Third Level

`, + }, + { + name: "h1 with existing id should keep it", + input: `

Heading with ID

`, + expected: `

Heading with ID

`, + }, + { + name: "h1 with user-content prefix should not double prefix", + input: `

Already Prefixed

`, + expected: `

Already Prefixed

`, + }, + { + name: "heading with special characters", + input: `

What is Wine Staging?

`, + expected: `

What is Wine Staging?

`, + }, + { + name: "heading with nested elements", + input: `

Bold and Italic

`, + expected: `

Bold and Italic

`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var result strings.Builder + ctx := NewTestRenderContext().WithEnableHeadingIDGeneration(true) + err := PostProcessDefault(ctx, strings.NewReader(tc.input), &result) + assert.NoError(t, err) + assert.Equal(t, tc.expected, strings.TrimSpace(result.String())) + }) + } +} + +func TestProcessNodeAttrID_SkipHeadingIDForComments(t *testing.T) { + // Test that HTML headings in comment-like contexts (issue comments) + // do NOT get auto-generated IDs to avoid duplicate IDs on pages with multiple documents. + // This is controlled by EnableHeadingIDGeneration which defaults to false. + testCases := []struct { + name string + input string + expected string + }{ + { + name: "h1 without id in comment context", + input: `

Heading without ID

`, + expected: `

Heading without ID

`, + }, + { + name: "h2 without id in comment context", + input: `

Another Heading

`, + expected: `

Another Heading

`, + }, + { + name: "h1 with existing id should still be prefixed", + input: `

Heading with ID

`, + expected: `

Heading with ID

`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var result strings.Builder + // Default context without EnableHeadingIDGeneration (simulates comment rendering) + err := PostProcessDefault(NewTestRenderContext(), strings.NewReader(tc.input), &result) + assert.NoError(t, err) + assert.Equal(t, tc.expected, strings.TrimSpace(result.String())) + }) + } +} diff --git a/modules/markup/render.go b/modules/markup/render.go index c645749065..12f002b0c6 100644 --- a/modules/markup/render.go +++ b/modules/markup/render.go @@ -54,6 +54,10 @@ type RenderOptions struct { // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page InStandalonePage bool + + // EnableHeadingIDGeneration controls whether to auto-generate IDs for HTML headings without id attribute. + // This should be enabled for repository files and wiki pages, but disabled for comments to avoid duplicate IDs. + EnableHeadingIDGeneration bool } // RenderContext represents a render context @@ -112,6 +116,11 @@ func (ctx *RenderContext) WithInStandalonePage(v bool) *RenderContext { return ctx } +func (ctx *RenderContext) WithEnableHeadingIDGeneration(v bool) *RenderContext { + ctx.RenderOptions.EnableHeadingIDGeneration = v + return ctx +} + func (ctx *RenderContext) WithUseAbsoluteLink(v bool) *RenderContext { ctx.RenderOptions.UseAbsoluteLink = v return ctx diff --git a/modules/migration/release.go b/modules/migration/release.go index f92cf25e7b..e25e7e4428 100644 --- a/modules/migration/release.go +++ b/modules/migration/release.go @@ -10,9 +10,12 @@ import ( // ReleaseAsset represents a release asset type ReleaseAsset struct { - ID int64 - Name string - ContentType *string `yaml:"content_type"` + ID int64 + Name string + + // There was a field "ContentType (content_type)" because Some forges can provide that for assets, + // but we don't need it when migrating, so the field is omitted here. + Size *int DownloadCount *int `yaml:"download_count"` Created time.Time diff --git a/modules/structs/pull.go b/modules/structs/pull.go index 7cc58217a0..3ad2f78bd3 100644 --- a/modules/structs/pull.go +++ b/modules/structs/pull.go @@ -140,6 +140,8 @@ type CreatePullRequestOption struct { Reviewers []string `json:"reviewers"` // The list of team reviewer names TeamReviewers []string `json:"team_reviewers"` + // Whether maintainers can edit the pull request + AllowMaintainerEdit *bool `json:"allow_maintainer_edit"` } // EditPullRequestOption options when modify pull request diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 9b1eb3fc85..09eeefd2c7 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -496,6 +497,11 @@ func CreatePullRequest(ctx *context.APIContext) { deadlineUnix = timeutil.TimeStamp(form.Deadline.Unix()) } + unitPullRequest, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests) + if err != nil { + ctx.APIErrorInternal(err) + } + prIssue := &issues_model.Issue{ RepoID: repo.ID, Title: form.Title, @@ -517,6 +523,8 @@ func CreatePullRequest(ctx *context.APIContext) { Type: issues_model.PullRequestGitea, } + pr.AllowMaintainerEdit = optional.FromPtr(form.AllowMaintainerEdit).ValueOrDefault(unitPullRequest.PullRequestsConfig().DefaultAllowMaintainerEdit) + // Get all assignee IDs assigneeIDs, err := issues_model.MakeIDsFromAPIAssigneesToAdd(ctx, form.Assignee, form.Assignees) if err != nil { diff --git a/services/migrations/github.go b/services/migrations/github.go index ae7350c016..ce631dcd42 100644 --- a/services/migrations/github.go +++ b/services/migrations/github.go @@ -329,7 +329,6 @@ func (g *GithubDownloaderV3) convertGithubRelease(ctx context.Context, rel *gith r.Assets = append(r.Assets, &base.ReleaseAsset{ ID: asset.GetID(), Name: asset.GetName(), - ContentType: asset.ContentType, Size: asset.Size, DownloadCount: asset.DownloadCount, Created: asset.CreatedAt.Time, diff --git a/services/migrations/gitlab.go b/services/migrations/gitlab.go index 260fa9cd5d..cbf974af2c 100644 --- a/services/migrations/gitlab.go +++ b/services/migrations/gitlab.go @@ -316,12 +316,11 @@ func (g *GitlabDownloader) convertGitlabRelease(ctx context.Context, rel *gitlab httpClient := NewMigrationHTTPClient() - for k, asset := range rel.Assets.Links { + for _, asset := range rel.Assets.Links { assetID := asset.ID // Don't optimize this, for closure we need a local variable r.Assets = append(r.Assets, &base.ReleaseAsset{ ID: int64(asset.ID), Name: asset.Name, - ContentType: &rel.Assets.Sources[k].Format, Size: &zero, DownloadCount: &zero, DownloadFunc: func() (io.ReadCloser, error) { diff --git a/services/migrations/main_test.go b/services/migrations/main_test.go index d0ec6a3f8d..581af614f9 100644 --- a/services/migrations/main_test.go +++ b/services/migrations/main_test.go @@ -171,7 +171,6 @@ func assertReactionsEqual(t *testing.T, expected, actual []*base.Reaction) { func assertReleaseAssetEqual(t *testing.T, expected, actual *base.ReleaseAsset) { assert.Equal(t, expected.ID, actual.ID) assert.Equal(t, expected.Name, actual.Name) - assert.Equal(t, expected.ContentType, actual.ContentType) assert.Equal(t, expected.Size, actual.Size) assert.Equal(t, expected.DownloadCount, actual.DownloadCount) assertTimeEqual(t, expected.Created, actual.Created) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index be6c4bdfd3..0c33227364 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -23420,6 +23420,11 @@ "description": "CreatePullRequestOption options when creating a pull request", "type": "object", "properties": { + "allow_maintainer_edit": { + "description": "Whether maintainers can edit the pull request", + "type": "boolean", + "x-go-name": "AllowMaintainerEdit" + }, "assignee": { "description": "The primary assignee username", "type": "string", diff --git a/tests/integration/api_pull_test.go b/tests/integration/api_pull_test.go index 433dce3d5e..0e1a88dcee 100644 --- a/tests/integration/api_pull_test.go +++ b/tests/integration/api_pull_test.go @@ -270,13 +270,20 @@ func TestAPICreatePullSuccess(t *testing.T) { owner11 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo11.OwnerID}) session := loginUser(t, owner11.Name) + prTitle := "test pull request title " + time.Now().String() token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner10.Name, repo10.Name), &api.CreatePullRequestOption{ Head: owner11.Name + ":master", Base: "master", - Title: "create a failure pr", + Title: prTitle, }).AddTokenAuth(token) MakeRequest(t, req, http.StatusCreated) + + // Also test that AllowMaintainerEdit is false by default + prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: prTitle}) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{IssueID: prIssue.ID}) + assert.False(t, pr.AllowMaintainerEdit) + MakeRequest(t, req, http.StatusUnprocessableEntity) // second request should fail } @@ -290,11 +297,14 @@ func TestAPICreatePullBasePermission(t *testing.T) { user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) session := loginUser(t, user4.Name) + prTitle := "test pull request title " + time.Now().String() token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) opts := &api.CreatePullRequestOption{ Head: repo11.OwnerName + ":master", Base: "master", - Title: "create a failure pr", + Title: prTitle, + + AllowMaintainerEdit: util.ToPointer(true), } req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner10.Name, repo10.Name), &opts).AddTokenAuth(token) MakeRequest(t, req, http.StatusForbidden) @@ -306,6 +316,11 @@ func TestAPICreatePullBasePermission(t *testing.T) { // create again req = NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner10.Name, repo10.Name), &opts).AddTokenAuth(token) MakeRequest(t, req, http.StatusCreated) + + // Also test that AllowMaintainerEdit is set to true + prIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{Title: prTitle}) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{IssueID: prIssue.ID}) + assert.True(t, pr.AllowMaintainerEdit) } func TestAPICreatePullHeadPermission(t *testing.T) {