Checkbox detection regexp misses some HTML comments

Summary

While working with a customer (internal link), we encountered a GitLab bug. The issue is a discrepancy between what is considered a HTML comment when displaying the formatted issue description, and what is considered a HTML comment when determining if a change to issue description was a task change (checkbox was toggled) or a text change. The outcome is that in certain specific cases, clicking a valid checkbox causes a history event that shows a diff of adding/removing the x instead of the expected "marked the checklist item" event.

Steps to reproduce

  1. Create a new issue (do not reuse an existing one),
  2. Set the description to this minimal example (note the missing space before the first -->):
<!-- comment-->
- [ ] task
<!-- comment -->
  1. Click the checkbox to mark the task as completed,
  2. Observe the created event showing a diff instead of the expected checklist item change.

Example Project

Reproduced on GitLab.com:

hmaraszek_ultimate_group/checkbox-test#1

What is the current bug behavior?

Clicking the checkbox creates an activity entry with the text "[user] changed the description", with a diff of the x being toggled.

What is the expected correct behavior?

Clicking the checkbox creates an activity entry with the text "[user] marked the checklist item [task] as [status]".

Relevant logs and/or screenshots

Observed:

image

Expected:

image

Output of checks

This bug happens on GitLab.com

Results of GitLab environment info

N/A

Results of GitLab application Check

N/A

Possible fixes

The determination whether an edit is a checkbox change or not happens in app/models/concerns/taskable.rb. It refers to a regexp that is ultimately sourced from lib/gitlab/regex.rb#L172. Here we can see that the regexp expects a space after the opening tag and before the closing tag. However, even without the spaces, the comments are invisible when rendered. The two behaviors should match - either by removing the spaces from the regexp above, or modifying the Markdown renderer to require the spaces. Personally, I believe the spaces should be removed from the regexp to match the behavior of web browsers when they encounter a HTML comment.

Edited by 🤖 GitLab Bot 🤖