Feature: Add the ability to @ mention users.

[FrostyWeasel]

No description provided.

[Martin092]

1700 line MR 😭😭😭

Can you explain what is going on here, what is this tiptap library?

Also rendering HTML from the user can be very dangerous due to all kinds of injection attacks, are you sure its safe? There are places in the code where you escape HTML manually, im not sure what is going on there but this does not feel like something that should be done by hand (bottom of content.ts).

[zakrok]

@Martin092 nice catch by the way,

yeah this is a no-go, implementing (open-source) HTML sanitizer is a recipe for disaster (like making our own cryptographic algorithm)

[FrostyWeasel]

Yeah, sorry @Martin092, I know this first push was a bit of a monolith, but I wasn't 100% sure if Tiptap would be worth it in the end and I really wanted something that could render on screen before I decided for certain that I would keep it.

Basically, Tiptap is a modular, headless text editor. It helps with allowing mentions to exist within the text areas for now, but in the future, it could be used to support italics, bulleted lists, code blocks, etc. The reason I wasn't sure if I wanted to keep it was that it felt a bit like over-engineering to wire this in if we were to only use it for mentions. However, upon looking into how to properly support mentions, it became clear that doing so with a bespoke implementation is quite a headache in its own right. So, between the pain of implementing my own mentioning system and integrating Tiptap, I chose the second because that development time at least gives us the option to easily integrate some of those extra features in the future if we choose to do so.

The core of wiring into our app basically boils down to three main pieces:

• The Editor (RichTextEditor.svelte & mentionSuggestion.ts): This is where a lot of the footprint comes from. It’s a Svelte wrapper around Tiptap to handle the actual input, plus the Tippy.js integration. Tiptap doesn't give us a mention UI out of the box, so this wires up the popup dropdown, handles keystroke navigation (like hitting Escape), and queries the user search API.
• The Read-Only View (RichTextRenderer.svelte): This was built purely for performance. Mounting a full Tiptap editor instance for every single comment in a long feed is unnecessary if it for viewing purposes only and not actual text editing. Instead, this bypasses the editor entirely and just uses Tiptap's generateHTML to convert the saved JSON into an HTML string which it then renders.
• Data & Schema (content.ts & tiptapExtensions.ts): This handles data integrity. Rather than just saving the plain text of a username (which could break if a user changes their handle later), this custom logic ensures we are saving their actual UUID into the database via the mention node's attributes, while still rendering the correct @username on the frontend.

On the HTML part: The escapeHTML related stuff is not actually unsafe, I believe. The only reason it is there is that the original intention was to have backwards compatibility with the plain text comments we have in the database. What it does is make sure that if we have something like "5 < 10" in a comment, we don't interpret the < as HTML, but instead make sure it is just the actual < character. It is therefore not meant to actually sanitize HTML, but to escape it. This point is a bit moot though, because as we discussed with Tom in the meeting, we don't actually need backwards compatibility. I was planning to remove this in favor of simplifying the code, rather than keeping full backwards compatible comment rendering when the comments currently in our database aren't needed anyway.

You are right to be worried about the fact that we need to render the comments via @html though. Tiptap needs it to be able to do stuff like: <p>Hello <strong>World</strong></p>. That means there is indeed an XSS vulnerability, but it is actually from Tiptap's generateHTML function where the result is actual HTML. I was looking into ways to sanitize this before rendering via @html and will probably use something similar to DOMPurify, but I am not fully done looking into it yet.