Skip to content

Commit

Permalink
Fix task list checkbox toggle to work with YAML front matter (go-gite…
Browse files Browse the repository at this point in the history
…a#25184) (go-gitea#25236)

Backport go-gitea#25184 by @jtran
Closes go-gitea#25225.

Fixes go-gitea#25160.

`data-source-position` of checkboxes in a task list was incorrect
whenever there was YAML front matter. This would result in issue content
or PR descriptions getting corrupted with random `x` or space characters
when a user checked or unchecked a task.
  • Loading branch information
jtran authored Jun 13, 2023
1 parent a9ebf91 commit 1650a26
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 7 deletions.
4 changes: 3 additions & 1 deletion modules/markup/markdown/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ func IsSummary(node ast.Node) bool {
// TaskCheckBoxListItem is a block that represents a list item of a markdown block with a checkbox
type TaskCheckBoxListItem struct {
*ast.ListItem
IsChecked bool
IsChecked bool
SourcePosition int
}

// KindTaskCheckBoxListItem is the NodeKind for TaskCheckBoxListItem
Expand All @@ -86,6 +87,7 @@ var KindTaskCheckBoxListItem = ast.NewNodeKind("TaskCheckBoxListItem")
func (n *TaskCheckBoxListItem) Dump(source []byte, level int) {
m := map[string]string{}
m["IsChecked"] = strconv.FormatBool(n.IsChecked)
m["SourcePosition"] = strconv.FormatInt(int64(n.SourcePosition), 10)
ast.DumpHelper(n, source, level, m, nil)
}

Expand Down
12 changes: 6 additions & 6 deletions modules/markup/markdown/goldmark.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
newChild := NewTaskCheckBoxListItem(listItem)
newChild.IsChecked = taskCheckBox.IsChecked
newChild.SetAttributeString("class", []byte("task-list-item"))
segments := newChild.FirstChild().Lines()
if segments.Len() > 0 {
segment := segments.At(0)
newChild.SourcePosition = rc.metaLength + segment.Start
}
v.AppendChild(v, newChild)
}
}
Expand Down Expand Up @@ -441,12 +446,7 @@ func (r *HTMLRenderer) renderTaskCheckBoxListItem(w util.BufWriter, source []byt
} else {
_, _ = w.WriteString("<li>")
}
_, _ = w.WriteString(`<input type="checkbox" disabled=""`)
segments := node.FirstChild().Lines()
if segments.Len() > 0 {
segment := segments.At(0)
_, _ = w.WriteString(fmt.Sprintf(` data-source-position="%d"`, segment.Start))
}
fmt.Fprintf(w, `<input type="checkbox" disabled="" data-source-position="%d"`, n.SourcePosition)
if n.IsChecked {
_, _ = w.WriteString(` checked=""`)
}
Expand Down
9 changes: 9 additions & 0 deletions modules/markup/markdown/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,22 @@ func actualRender(ctx *markup.RenderContext, input io.Reader, output io.Writer)
}
buf = giteautil.NormalizeEOL(buf)

// Preserve original length.
bufWithMetadataLength := len(buf)

rc := &RenderConfig{
Meta: "table",
Icon: "table",
Lang: "",
}
buf, _ = ExtractMetadataBytes(buf, rc)

metaLength := bufWithMetadataLength - len(buf)
if metaLength < 0 {
metaLength = 0
}
rc.metaLength = metaLength

pc.Set(renderConfigKey, rc)

if err := converter.Convert(buf, lw, parser.WithContext(pc)); err != nil {
Expand Down
37 changes: 37 additions & 0 deletions modules/markup/markdown/markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,3 +519,40 @@ func TestMathBlock(t *testing.T) {

}
}

func TestTaskList(t *testing.T) {
testcases := []struct {
testcase string
expected string
}{
{
// data-source-position should take into account YAML frontmatter.
`---
foo: bar
---
- [ ] task 1`,
`<table>
<thead>
<tr>
<th>foo</th>
</tr>
</thead>
<tbody>
<tr>
<td>bar</td>
</tr>
</tbody>
</table>
<ul>
<li class="task-list-item"><input type="checkbox" disabled="" data-source-position="19"/>task 1</li>
</ul>
`,
},
}

for _, test := range testcases {
res, err := RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase)
assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase)
}
}
3 changes: 3 additions & 0 deletions modules/markup/markdown/renderconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ type RenderConfig struct {
TOC bool
Lang string
yamlNode *yaml.Node

// Used internally. Cannot be controlled by frontmatter.
metaLength int
}

// UnmarshalYAML implement yaml.v3 UnmarshalYAML
Expand Down
8 changes: 8 additions & 0 deletions web_src/js/markup/tasklist.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ export function initMarkupTasklist() {

const encoder = new TextEncoder();
const buffer = encoder.encode(oldContent);
// Indexes may fall off the ends and return undefined.
if (buffer[position - 1] !== '['.codePointAt(0) ||
buffer[position] !== ' '.codePointAt(0) && buffer[position] !== 'x'.codePointAt(0) ||
buffer[position + 1] !== ']'.codePointAt(0)) {
// Position is probably wrong. Revert and don't allow change.
checkbox.checked = !checkbox.checked;
throw new Error(`Expected position to be space or x and surrounded by brackets, but it's not: position=${position}`);
}
buffer.set(encoder.encode(checkboxCharacter), position);
const newContent = new TextDecoder().decode(buffer);

Expand Down

0 comments on commit 1650a26

Please sign in to comment.