MediaEmbed: Set custom element name

[tony]

See #9373 for details.

<oembed> is an invalid name for custom elements (WhatWG, MDN).

Allow setting element name (especially to <o-embed>). Retain compatibility with existing tags.

Suggested merge commit message (convention)

Type: Message. Closes #9373

Checklist

MediaEmbed

• Add preferredElementName option (default: 'oembed')
• Add elementNames option (default: ['oembed', 'o-embed'])
• Preserves existing behavior
  • elementNames option: Default includes 'oembed': ['oembed', 'o-embed']
  • preferredElementName option: Default is 'oembed'
• Basic docs (Internal API)
• Basic docs (Markdown docs)
• Tests

---

Additional information

Also related: #2737 (Implement a web component to handle <oembed url="...">)

I have an <o-embed> custom element polyfill here: Github, NPM CodeSandbox

[Reinmar]

Wow, this is super complete :) Thank you 😍

I'll check who could review this on our side.

As for the solution – initially, I wasn't sure whether making this configurable is the right direction. If this feature's output is configurable this way then why not every other's? We need to be cautious in such cases.

However, I think we should ask ourselves why this particular feature got this PR and no other. I think the answer is that it uses a non-standard HTML output and hence there's no convention we're able to follow (BTW, I wouldn't go with o-embed too as why would the o be separated from the rest of the name? what about features that are undividable single-word?). In such case, making the output configurable makes more sense, even though it's always modifiable via custom converters.

One thing that I'd consider simplifying in the PR is whether exposing elementNames really makes sense. What if the feature only handles oembed + what it was configured to use in its output (config.mediaEmbed.elementName)? It's potentially less future-proof but also simpler.

[Reinmar]

Wow, this is super complete :) Thank you 😍

I'll check who could review this on our side.

As for the solution – initially, I wasn't sure whether making this configurable is the right direction. If this feature's output is configurable this way then why not every other's? We need to be cautious in such cases.

However, I think we should ask ourselves why this particular feature got this PR and no other. I think the answer is that it uses a non-standard HTML output and hence there's no convention we're able to follow (BTW, I wouldn't go with o-embed too as why would the o be separated from the rest of the name? what about features that are undividable single-word?). In such case, making the output configurable makes more sense, even though it's always modifiable via custom converters.

One thing that I'd consider simplifying in the PR is whether exposing elementNames really makes sense. What if the feature only handles oembed + what it was configured to use in its output (config.mediaEmbed.elementName)? It's potentially less future-proof but also simpler.

[tony]

What if the feature only handles oembed ...

oembed is not a valid element name (WhatWG, MDN). In my opinion, CKEditor's MediaEmbed is rendering invalid HTML.

If you try to build a web component with oembed instead of o-embed (or something hyphenated), here's what most linters will show:

./src/OEmbedElement.ts

    The tag name 'oembed' is not a valid custom element name. Remember that a hyphen (-) is required.
    26:  @customElement('oembed')
    no-invalid-tag-name

That's why my PR is opinionated toward phasing it out, as it's introducing URL that wouldn't work with custom elements.

One thing that I'd consider simplifying in the PR is whether exposing elementNames really makes sense. What if the feature only handles oembed + what it was configured to use in its output (config.mediaEmbed.elementName)? It's potentially less future-proof but also simpler.

Created alternative PR at #9418, without a public elementNames, and where 'oembed' is implicitly supported.

The downside is oembed can't be omitted or phased out without a custom build. It's an edgecase, but I wanted the config to be able to do it without needing a specialized build/fork. From a user standpoint: I'm flexible of course! Generally I think making things available via config means not requiring a rebuild of the editor. That's a win for the user - at the expense of complication / (if taken too far, bloat) in the code itself.

If there's any renaming/bikeshedding - I'm happy to make adjustments! (e.g. renamed preferredElementName -> elementName)

[psmyrek]

I also think that solution for this issue could be simplified to a form, where media embed feature supports the existing <oembed> element plus the configurable one via config.mediaEmbed.elementName. I think there is no need to expose the list of supported view elements.

So lets move to your 2^nd PR: #9418. I'm closing this one.

[tony]

This PR continues at #9418