I'll revise my thinking from 8 years ago.

GLFM has come a long way since then. I think this would be the time to start embracing generic directives. In particular, Container Block Directives. It's not an official syntax, but it has been adopted in a few different projects, such as micromark / remark, docusarus for their admonitions, Alt Text Editor for Confluence, and Myst Markdown, which I had never heard of before.

In a nut shell, it provides a way to specify different types of markdown extensions using a consistent format, particularly

:::extension-name

something

:::

Myst markdown actually supports a set of tabs, and some other cool structures like grids and cards. Though personally, I'm not crazy about their use of curly braces

:::{extension-name}

But I could certainly see a directive for a set of tabs, like :::tabs, and a specific tab like :::tab. For example something like

::::tabs Instructions for different languages

:::tab Ruby

When writing Ruby use

```ruby
x = 1
```

:::

:::tab Javascript

When writing Javascript use

```javascript
let x = 1;
```
:::
::::

Attributes would be attached like :::tab {.active}, etc, like is currently done for width and height on images.

This would then allow us to add additional extensions eventually in a consistent manner.

However, a few problems with this syntax:

• it doesn't degrade well - normal markdown doesn't know what to do with :::, so it would not render in a readable fashion outside GitLab.
• no consistency between extension names and different markdown flavors. The obvious example is Myst markdown, which uses :::{tab-set}, whereas I think either :::tabs or :::tab-set is better.
• extension names can't be localized. One of the huge discussion points of GitHub's alerts was the fact that the use of the alert names in the syntax, such as warning, was in English. Rather than using a purely markdown style syntax of just special characters. While I think that is important for base level functionality, and when possible should be followed, it's just not feasible when supporting a wide range of possible extensions.

Some food for thought and research...

This is indeed a great idea. In fact there has been an issue open on it for quite awhile as well: Tabbed content for GitLab Flavored Markdown (GFM) (#25399)

The difficulty has always been exactly what syntax to use. Feel free to add any ideas/discussion to that issue, maybe it will get revived.

/cc @kivikakk @mmacfarlane

So far so good. Just on my way back from Iceland to take pictures of this

Hmm, does the existing syntax for height/width on media embeds work?

@mmacfarlane @kivikakk I think this ca be closed. We added “Collapsible section” in the extra menu for both editors right before I left.

I haven't looked at the latest code, but we do have the ability to show an alert/error message when rendering mermaid blocks, such as if the character limit is maxed out, or there are too many blocks and we limit rendering unless the user clicks a box. This is done in app/assets/javascripts/behaviors/markdown/render_sandboxed_mermaid.js. Maybe it helps.

I agree with @kivikakk and @brendan777 that it would certainly be more user-friendly to adjust the url if we need to. The other thing that would allow is if the urls ever change in the future, we'll be able to add the additional rewrite.

@kivikakk hey, I got the notification that this was idle for 12 months. This might be fixed now with the work the transition to the comrak task lists 🤞

Another way to tackle it is adding support in the rich text editor to paste an iframe embed, and a dialog that gets thrown up. For example, when you click on the "Insert link" toolbar icon, a little dialog is shown allowing the user to type in the text and url for the link. Something similar could be done for this, where there are two fields and you can either paste just the embed link or a full iframe embed (or something, haven't really thought about it). Since we convert back into GLFM, that would be converted to our syntax, along with the width and height attributes added if specified. Just a thought.

Either way, we should probably add an "Iframe" item to the "More options" menu for both plain and rich text edtiors.

@slashmanov @kivikakk @clavimoniere

Warning, long post

Regarding direct use of the <iframe> tag:

I think I summed it up in !200864 (comment 2693537003)

...we do support embedding video and audio using the markdown image syntax. So you can do ![](http://mirrors.standaloneinstaller.com/video-sample/small.mp4) and it gives you the video player.

The key is we detect the media type in the image link, and insert the proper tag, <video> or <audio>. It also automatically handles using the asset proxy if it's enabled.

I'm just not crazy about allowing the <iframe> tag in the markdown. We don't allow <video> or <audio>, I don't think we should allow <iframe> either.

We already consider the image syntax, ![]() to be "media" embed, and add the proper tags ourselves. By using image syntax, we can control exactly how the iframe is embedded, and standardize on specific attributes. Width and height are already supported on our image syntax, so that's not a problem.

There are several points that I took into consideration:

1. Safety is number one. While we do allow some direct HTML tags, we don't allow them all. <details> is one of those that we began allowing (I seem to remember that it was originally sanitized out 🤷), but it doesn't have any security concerns, as long as we clearly document how to use it properly.

   We sanitize aggressively, as we should. We don't allow direct usage of <audio> or <video> tags, even though we support the embedding of audio and video. We leveraged the markdown image syntax to detect audio/video, and insert the tags the way we want, with only the attributes we want to support. In fact we added support in the markdown syntax to allow the specifying of dimensions for video and images. I find it's easier to sanitize out any potential dangerous things (whitelisting) and then support exactly what we want in a controlled fashion. We don't support CSS or style attributes either, with the intent that the height/width syntax will (someday) get extended to support specific dedicated CSS that we control.

2. Whenever we contemplate adding some additional support in our markdown rendering, I typically spent a lot of time and thought trying to find the best way to fit into the markdown ecosystem. Not simply adding stuff here and there. The communication syntax in GitLab is GLFM, not HTML. So it is(was) important to me to add support for something in the most generally accepted way in the markdown ecosystem. Unfortunately CommonMark has yet to properly solve many of the "extension syntax" issues, such as embedding of media or specifying attributes on objects. So it's a lot of reading everyone's opinion on the CommonMark forum, and inspiration from the author of CommonMark, John MacFarlane, in his implementation of both pandoc and commonmark-hs, which is where I settled on the attribute syntax.

   Our syntax is GLFM, not HTML, so that should be the main focus in how we add support for something new. Whether it's support for Mermaid and other diagrams, or math, multiline block quotes, etc.

   That's a long way of saying that consistency, to me, is super important. We already supported a syntax for embedding audio and video. So I felt that was the best way to add support for iframes. So if you know how to embed one piece of media, you know how to embed the others, using the same syntax and attribute support.

3. In true GitLab fashion, I always tried to go for the smallest implementation, and across multiple MRs when possible. Adding the IframeLinkFilter Banzai filter in !204952 (merged) fit that. It leverages our PlayableLinkFilter filter, which audio and video use as well.

   We also use iframes to insert mermaid diagrams (and even control how many at a time are displayed). This also fits directly into that model.

   Adding direct <iframe> support felt messy and more complicated, inconsistent, and unnecessary at this stage.

4. Currently, AFAIK, we don't use any Web Components in our rendering of our markdown. Maybe that is a direction you all will go in eventually, but I'm not convinced that is what is needed here.

   Remember also that there are other consumers of our rendered HTML, both the API and our content editor. So have to be considered.

5. While I understand that being able to just copy/paste a full iframe tag feels desirable, it doesn't really fit, in my opinion, well in with how we do things.

   I think using the consistent syntax as the first implementation would be the way to go, and then consider raw iframe tag support later if there was a real need. But as I said, we don't in general allow full tag support for audio and video and all their attributes, so I think it would need some significant justification to go outside that model.

6. The use of a <markdown-iframe> might be interesting, rather than using <img class="js-render-iframe">, although again that fits in with how we currently handle such things, and provides a reasonable fallback for an API consumer. Don't know if <markdown-iframe> would provide that.

   You could still use the existing frontend display method (using the new tag) without introducing a web component just yet 🤷. Iteration

As mentioned, I'm no longer with GitLab and thus left it in the capable hands of @kivikakk. Just providing some context and history. But I'm always happy to clarify my thinking, right or wrong, when needed.

@slashmanov regarding !200864 (comment 2873222661) about domain format in the application settings

I'm not at all against changing this, but the design has been discussed a lot already and so many drafts made that I'd like for such changes to be made in future iterations, to land before the feature flag is removed.

My biggest worry is that if we ship it that way (even behind a feature flag) that change would be impossible to roll back since people would start to depend on it and we will have to keep it as-is to preserve compatibility with existing notes. This might introduce issues when you have both images and iframes on the same domain. Worse than that these would be transitive issues: images work fine initially but break after editing.

I disagree with there being any worry about preserving customer use of it (other than maybe GitLab). I purposely used a wip feature flag in !204952 (diffs). This means if a customer happens to use it, we are not on the hook to support, its data format, or whether the feature doesn't eventually get removed completely. I was not yet completely confident on the format, and was intending on rolling this out only internally to help test it out and better solidify our implementation. Over cautious maybe, but considering the potential security problems around iframes, and the rise of other security issues, I felt it would be prudent.

@clavimoniere Yeah, as an old-timer, I get to keep my original username, which was re-instated after a brief period. So I'll try to provide context when I can.

@rliu-gl @kivikakk This change is not all necessary. @clavimoniere added it into his initial MR. My guess is that he saw this could be changed while he was in this area of the code, and since he moved the Kroki setting panel to a slightly different location/order. Ideally it would have been extracted into its own MR in the spirit of keeping MRs as small as possible. But it's not obvious when you're relatively new to the code.

In the most-requested examples, if someone were to directly embed an image from www.youtube.com or embed.figma.com, this would embed them in an iframe instead.

Just for context, my initial thinking on this was that, at least for the YouTube case, one would whitelist their embedding url, such as https://www.youtube.com/embed. I don't know if images can be pulled from that url, or if we would even need to support that. I look at embed as saying iframe

Can't speak for Figma embeds, but embed.figma.com also sounds fairly clear. But it should certainly be revisited if it needs to be.

Just one thing though, I would suggest considering breaking MRs like this into smaller ones in the future. It helps make it easier to review the changes (less intimidating as well 😆). @digitalmoksha mentioned this as well.

Yeah, not really much more that could have been split out. The only thing I see would have been to put the DB migration in its own MR, and the Kroki fixes could have been its own MR. But those don't reduce complexity that much.

The issue with subgraph titles overlap is being tracked in https://github.com/mermaid-js/mermaid/issues/3806

Ah, that's been there since 2022, so we shouldn't let that stop it from release.

@kivikakk wow, really good investigation and cleanup. This should significantly help. 🚀

You might consider adding the label Category:Markdown to any MRs and issues related to this area, to help with categorization and discovery.

Thanks @kivikakk for pointing this out - this would be a nice update. It would be great to get the bug fixed 🚀