Closed Bug 1456878 (bmo-markdown-initial-release) Opened 2 years ago Closed Last year

Support markdown comments

Categories

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

Development
enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dylan, Assigned: imadueme)

References

(Blocks 3 open bugs)

Details

(Keywords: bmo-goal, bmo-ux)

Attachments

(1 file, 1 obsolete file)

45 bytes, text/x-github-pull-request
Details | Review
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)
Alias: bmo-markdown
Depends on: bmo-libcmark-gfm
Keywords: bmo-ux
Depends on: bmo-link-preview
Assignee: nobody → 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.
(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.
(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.
Depends on: bmo-markdown-style
Depends on: bmo-markdown-model
Attached file Github PR #621 (obsolete) —
See Also: → 1373420
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Blocks: 1482447
Depends on: 1482474
PREVIOUSLY: *How is this fixed?*  It does not appear to work.
See Comment 6. Thank you :kohei.
Flags: needinfo?(dylan)
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.
Status: RESOLVED → REOPENED
Flags: needinfo?(dylan)
Resolution: FIXED → ---
No longer blocks: 1373420
Based on github, will this support tables?  The video demo says "yes"
Yep, it does. And it's so close to being in production.
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.
Attached file GitHub Pull Request
Attachment #8989155 - Attachment is obsolete: true
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → FIXED
Depends on: 1518243
Blocks: 1518266
Blocks: 1518350
Blocks: 1518499
Blocks: 1518338
Blocks: 1518268
No longer depends on: 1518268
Blocks: 1512497
No longer depends on: 1512497
Blocks: 1518218
No longer depends on: 1518218
Blocks: 1518264
No longer depends on: 1518264
Blocks: 1518303
No longer depends on: 1518303
Blocks: 1518266
No longer depends on: 1518266
Blocks: 1518463
No longer depends on: 1518463
Blocks: 1518304
No longer depends on: 1518304
Blocks: 1518276
No longer depends on: 1518276
Blocks: 1518199
No longer depends on: 1518199
Alias: bmo-markdown → bmo-markdown-initial-release
Blocks: bmo-markdown
No longer blocks: 1474153
No longer blocks: 1352571
Blocks: 1518967
Blocks: 1525376
No longer blocks: 1518218
Regressions: 1518218
Regressions: 1521423
Blocks: 1240472
You need to log in before you can comment on or make changes to this bug.