Avoid iframes having to deal with asset proxy/lazy load

What does this MR do and why?

Part of <iframe> embeds in Markdown content (#282443).

Right now, iframes emitted by GitLab's IframeLinkFilter (i.e. entirely under our control, 100% safe!) still have to do a lot of backflipping to avoid getting wrecked by two other components:

  • AssetProxyFilter, which applies to images if the asset proxy is configured and enabled. Iframes look like images before being transformed by IframeLinkFilter, so they get "asset proxied" and we have to account for this possibility in the frontend. (This actually slipped our collective mind during the first roll-out of this FF'd feature, and was part of the first post-deploy fix of this to GitLab.com to get it actually working — see last paragraph of the "What does this MR do and why?" section of !215529 (merged).)
  • ImageLazyLoadFilter, which applies to all images. We have to account for this too, while also accounting for the possibility that it may be disabled on a self-managed instance.

This ends up complicating the filter landscape a fair bit — there are 4 combinations of these two filters being enabled or disabled — and makes building additional functionality on the iframe loading (as proposed in #282443) quite difficult. This is all mostly a consequence of where in GfmPipeline the IframeLinkFilter was initially placed — it's handled after all these other filters, so it has no choice but to deal with the consequences of these elements having already been handled without any inkling they might be handled as iframes later.

This MR shifts the "iframe recognition" filter, IframeLinkFilter, up before these other two. This means successful iframe candidates are marked with js-render-iframe before the other filters run, and the other filters can then ignore such elements. As a result, the frontend doesn't have to deal with the possibility of the correct iframe src being one of three (!) different attributes.

We also have to bring AttributesFilter up — IframeLinkFilter depends on it having already run, so we need to preserve its relative ordering to IframeLinkFilter. (Opened Have Banzai filters declare their dependency or... (#591853) to automate the validation of this requirement.)

References

Past work:

Present work: a series of 4 stacked MRs for %18.10 to bring this closer to GA:

How to set up and validate locally

This is entirely a refactor, with zero observable behaviour difference. Here's how it should work:

  1. Check out the branch.
  2. Enable the allow_iframes_in_markdown feature on your GDK.
  3. Enable embedding of www.youtube.com from /admin/application_settings/general#js-iframe-settings, and ensure Enable embedded content is turned on.
  4. It may be wise to restart the GDK Rails instance here, for best chances of success.
  5. Using the plain-text editor, insert content like seen in the screenshot above to a wiki page, issue description, MR comment, etc.:
    Here's my YouTube video:
    
    ![](https://www.youtube.com/embed/DwIepRJgMF4){width=800 height=600}
  6. It should look like a big embedded YouTube video!

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Asherah Connor

Merge request reports

Loading