Make TaskCheckBox render correctly (#11214)

* Fix checkbox rendering

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Normalize checkbox rendering

Signed-off-by: Andrew Thornton <art27@cantab.net>

* set the checkboxes to readonly instead of disabled

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lauris BH <lauris@nix.lv>

modules/markup/markdown/ast.go

@@ -4,7 +4,11 @@
 
 package markdown
 
-import "github.com/yuin/goldmark/ast"
+import (
+	"strconv"
+
+	"github.com/yuin/goldmark/ast"
+)
 
 // Details is a block that contains Summary and details
 type Details struct {
@@ -70,6 +74,41 @@ func IsSummary(node ast.Node) bool {
 	return ok
 }
 
+// TaskCheckBoxListItem is a block that repressents a list item of a markdown block with a checkbox
+type TaskCheckBoxListItem struct {
+	*ast.ListItem
+	IsChecked bool
+}
+
+// KindTaskCheckBoxListItem is the NodeKind for TaskCheckBoxListItem
+var KindTaskCheckBoxListItem = ast.NewNodeKind("TaskCheckBoxListItem")
+
+// Dump implements Node.Dump .
+func (n *TaskCheckBoxListItem) Dump(source []byte, level int) {
+	m := map[string]string{}
+	m["IsChecked"] = strconv.FormatBool(n.IsChecked)
+	ast.DumpHelper(n, source, level, m, nil)
+}
+
+// Kind implements Node.Kind.
+func (n *TaskCheckBoxListItem) Kind() ast.NodeKind {
+	return KindTaskCheckBoxListItem
+}
+
+// NewTaskCheckBoxListItem returns a new TaskCheckBoxListItem node.
+func NewTaskCheckBoxListItem(listItem *ast.ListItem) *TaskCheckBoxListItem {
+	return &TaskCheckBoxListItem{
+		ListItem: listItem,
+	}
+}
+
+// IsTaskCheckBoxListItem returns true if the given node implements the TaskCheckBoxListItem interface,
+// otherwise false.
+func IsTaskCheckBoxListItem(node ast.Node) bool {
+	_, ok := node.(*TaskCheckBoxListItem)
+	return ok
+}
+
 // Icon is an inline for a fomantic icon
 type Icon struct {
 	ast.BaseInline

modules/markup/markdown/goldmark.go

@@ -10,7 +10,6 @@ import (
 	"regexp"
 	"strings"
 
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/markup/common"
 	"code.gitea.io/gitea/modules/setting"
@@ -129,6 +128,21 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
 			if v.HasChildren() && v.FirstChild().HasChildren() && v.FirstChild().FirstChild().HasChildren() {
 				if _, ok := v.FirstChild().FirstChild().FirstChild().(*east.TaskCheckBox); ok {
 					v.SetAttributeString("class", []byte("task-list"))
+					children := make([]ast.Node, 0, v.ChildCount())
+					child := v.FirstChild()
+					for child != nil {
+						children = append(children, child)
+						child = child.NextSibling()
+					}
+					v.RemoveChildren(v)
+
+					for _, child := range children {
+						listItem := child.(*ast.ListItem)
+						newChild := NewTaskCheckBoxListItem(listItem)
+						taskCheckBox := child.FirstChild().FirstChild().(*east.TaskCheckBox)
+						newChild.IsChecked = taskCheckBox.IsChecked
+						v.AppendChild(v, newChild)
+					}
 				}
 			}
 		}
@@ -221,11 +235,11 @@ func (r *HTMLRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) {
 	reg.Register(KindDetails, r.renderDetails)
 	reg.Register(KindSummary, r.renderSummary)
 	reg.Register(KindIcon, r.renderIcon)
+	reg.Register(KindTaskCheckBoxListItem, r.renderTaskCheckBoxListItem)
 	reg.Register(east.KindTaskCheckBox, r.renderTaskCheckBox)
 }
 
 func (r *HTMLRenderer) renderDocument(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
-	log.Info("renderDocument %v", node)
 	n := node.(*ast.Document)
 
 	if val, has := n.AttributeString("lang"); has {
@@ -311,24 +325,42 @@ func (r *HTMLRenderer) renderIcon(w util.BufWriter, source []byte, node ast.Node
 	return ast.WalkContinue, nil
 }
 
-func (r *HTMLRenderer) renderTaskCheckBox(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
-	if !entering {
-		return ast.WalkContinue, nil
+func (r *HTMLRenderer) renderTaskCheckBoxListItem(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
+	n := node.(*TaskCheckBoxListItem)
+	if entering {
+		n.Dump(source, 0)
+		if n.Attributes() != nil {
+			_, _ = w.WriteString("<li")
+			html.RenderAttributes(w, n, html.ListItemAttributeFilter)
+			_ = w.WriteByte('>')
+		} else {
+			_, _ = w.WriteString("<li>")
 		}
-	n := node.(*east.TaskCheckBox)
-
 		end := ">"
 		if r.XHTML {
 			end = " />"
 		}
 		var err error
 		if n.IsChecked {
-		_, err = w.WriteString(`<span class="ui fitted disabled checkbox"><input type="checkbox" disabled="disabled"` + end + `<label` + end + `</span>`)
+			_, err = w.WriteString(`<span class="ui checked checkbox"><input type="checkbox" checked="" readonly="readonly"` + end + `<label>`)
 		} else {
-		_, err = w.WriteString(`<span class="ui checked fitted disabled checkbox"><input type="checkbox" checked="" disabled="disabled"` + end + `<label` + end + `</span>`)
+			_, err = w.WriteString(`<span class="ui checkbox"><input type="checkbox" readonly="readonly"` + end + `<label>`)
 		}
 		if err != nil {
 			return ast.WalkStop, err
 		}
+		fc := n.FirstChild()
+		if fc != nil {
+			if _, ok := fc.(*ast.TextBlock); !ok {
+				_ = w.WriteByte('\n')
+			}
+		}
+	} else {
+		_, _ = w.WriteString("</label></span></li>\n")
+	}
+	return ast.WalkContinue, nil
+}
+
+func (r *HTMLRenderer) renderTaskCheckBox(w util.BufWriter, source []byte, node ast.Node, entering bool) (ast.WalkStatus, error) {
 	return ast.WalkContinue, nil
 }

modules/markup/sanitizer.go

@@ -42,7 +42,7 @@ func ReplaceSanitizer() {
 
 	// Checkboxes
 	sanitizer.policy.AllowAttrs("type").Matching(regexp.MustCompile(`^checkbox$`)).OnElements("input")
-	sanitizer.policy.AllowAttrs("checked", "disabled").OnElements("input")
+	sanitizer.policy.AllowAttrs("checked", "disabled", "readonly").OnElements("input")
 
 	// Custom URL-Schemes
 	sanitizer.policy.AllowURLSchemes(setting.Markdown.CustomURLSchemes...)
@@ -57,7 +57,11 @@ func ReplaceSanitizer() {
 	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`task-list`)).OnElements("ul")
 
 	// Allow icons
-	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^icon(\s+[\p{L}\p{N}_-]+)+$`)).OnElements("i", "span")
+	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^icon(\s+[\p{L}\p{N}_-]+)+$`)).OnElements("i")
+	sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^((icon(\s+[\p{L}\p{N}_-]+)+)|(ui checkbox)|(ui checked checkbox))$`)).OnElements("span")
+
+	// Allow unlabelled labels
+	sanitizer.policy.AllowNoAttrs().OnElements("label")
 
 	// Allow generally safe attributes
 	generalSafeAttrs := []string{"abbr", "accept", "accept-charset",