1456878 - (bmo-markdown-initial-release) Support markdown comments

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, enhancement)

Description

[Dylan Hardison [:dylan] (he/him)]

In 2018 all bugzilla comments should be rendered as markdown.

Basic outline of this feature:

- We use the common markdown reference implementation (libcmark / CommonMark (perl binding))
- All new comments get rendered as markdown.
- Existing auto-linking continues to work with little or no modification (e.g. bug NNNN autolinks)

Comment 1

[Israel Madueme [:imadueme]]

Just some things I've learned while working on this that I wanted to record.

After bmo-libcmark-gfm bmo-link-preview land I think we'll need to:

- Disable autolinking url looking things in quoteUrls2
- Not use a pre tag for rendered markdown comments :)
- Add some new css styling for markdown comments. The html tags outputed by markdown are styled by the overall bmo site rules, and because of that look pretty ugly. Things are too big, others have too much spacing around them, etc.
- Investigate how we will differenciate between an old plaintext comment and a markdown comment.
- Investigate adding a UI editor to help people not familiar with markdown syntax.

Comment 2

[Mark Côté [:mcote]]

(In reply to Israel Madueme [:imadueme] from comment #1)
> - Investigate adding a UI editor to help people not familiar with markdown
> syntax.

This can come later as long as we document the syntax and provide a convenient link.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

(In reply to Israel Madueme [:imadueme] from comment #1)
> Just some things I've learned while working on this that I wanted to record.
> 
> After bmo-libcmark-gfm bmo-link-preview land I think we'll need to:
> 
> - Disable autolinking url looking things in quoteUrls2

Quite possibly, renaming quoteUrls2 to render_markdown(), and the original quoteUrls() rename to render_classic();

> - Investigate how we will differenciate between an old plaintext comment and
> a markdown comment.

There's already a database column for this (is_markdown), but it needs to re-added to the schema and the Bugzilla::Comment
object. Ask dkl for details.

All new comments should default to markdown, with no user option to use the "classic" rendering.

Comment 4

[Israel Madueme [:imadueme]]

Attachment: Github PR #621

Comment 5

[IdmFoundInHim]

PREVIOUSLY: *How is this fixed?*  It does not appear to work.
See Comment 6. Thank you :kohei.

Comment 6

[Kohei Yoshino [:kohei]]

Remember that RESOLVED FIXED on this Bugzilla generally means the patch has been merged to master/trunk and does _not_ necessarily mean the feature/fix has been deployed to production*. Anyway, the pull request here has been backed out due to several issues so the bug is still open.

* Depending on product and component. Sometimes VERIFIED FIXED = deployed.

Comment 7

[Albert Ting]

Based on github, will this support tables?  The video demo says "yes"

Comment 8

[Dylan Hardison [:dylan] (he/him)]

Yep, it does. And it's so close to being in production.

Comment 9

[Dylan Hardison [:dylan] (he/him)]

Super minimalism time. We're going to stop caring about a few things:

1) The user story. It can stay as is for now
2) Comments on the non-modal show bug page
3) the accuracy of preloading bugs.
4) Adding nofollow to outgoing links.

We can care about each of those at a later date (and probably #3 first).

Take the existing PR, drop all the changes to quoteUrls() and all the call sites of quoteUrls(), *except*
for the preview API and the call to quoteUrls() that is part of the BugModal page.
If there is a mistake in the markdown code that allows XSS, it will be blocked by the CSP on the modal pages.
For this reason, we can't support it on the old bug view.

For rendering the comments, take the code in https://github.com/mozilla-bteam/bmo/pull/885.
That code calls quoteUrls() on the body text of paragraphs and list items and I think the behavior should be as expected.

In addition, in that render_html() method, you can use Mojo::DOM to remove style attributes and on*event handlers.

Comment 10

[Israel Madueme [:imadueme]]

Attachment: GitHub Pull Request