ckeditor4
ckeditor4 copied to clipboard
check if media embed should try to handle all URLs or only selected that it supports
Type of report
Feature
Provide detailed reproduction steps (if any)
- copy url from the browser,
- paste into the editor
Expected result
text as a link
Actual result
Other details
- Browser: Chrome, Firefox
- OS: OSX
- CKEditor version: demo on the ckeditor website
- Installed CKEditor plugins: ?
It looks like the Media Embed plugin is too aggressive and tries to handle more URLs that it is able to properly embed. It may be also that iframe.ly integration got broken at some point :/
IIRC it used to work better, for example when a link to Amazon product was placed IIRC the product was nicely embeded (but I may be wrong).
Makes a perfect sens, though I'll make it a feature as all in all that's an addition to current plugin.
Seems like Iframe.ly has new feature: support for OpenGraph Protocol → https://iframely.com/embed/https%3A%2F%2Fwww.comandeer.pl%2F
Yep, Amazon:
iframe.ly demo:
Iframe.ly in CKEditor:
That's much probably due to <script async src="//if-cdn.com/embed.js" charset="utf-8"></script>
, that probably finds elements with data-iframely-url
.
Yup, I can confirm that ☝️. The problem is that this script inserts an iframe
into the content, which is something that we'd like to avoid.
There are some info that allows us to recreate the content in some extend, without throwing an iframe, and relay on 3rd party.
However it is missing some information, like a favicon. But we have title, description, UTL and thumbnail - which is enough to create some meaningful media out of it.
However it is missing some information, like a favicon. But we have title, description, UTL and thumbnail - which is enough to create some meaningful media out of it.
Agree, with @mlewand, since iframely
supports it and returns some info it will be good to try to utilize that instead of just blocking some type of responses.
We insert response.html
into editor, which contains two things. First is the placeholder - a preview, in some cases it looks very close to what final content would look like, and in some cases it looks completely different. Second part is a script, that replaces placeholder with an iframe. But we are removing the script.
How it looks now
Spotify
What you see here is just an img that comes with
response.html
https://ckeditor.com/ckeditor-4/demo/
This is also an img from
response.html
.
How it might look when we ignore returned html, and create content by our own
Spotify
https://ckeditor.com/ckeditor-4/demo/
Note: styling is to be improved.
Responses
Spotify
{
'url': 'https://open.spotify.com/track/2XsTC2dPbmWREu5RQ1mQC0',
'type': 'rich',
'version': '1.0',
'title': 'Comfortably Numb - Live',
'author': 'Pink Floyd',
'author_url': 'https://open.spotify.com/artist/0k17h0D3J5VfsdmQ1iZtE9',
'provider_name': 'Spotify',
'description': 'Comfortably Numb - Live, a song by Pink Floyd on Spotify',
'thumbnail_url': 'https://i.scdn.co/image/482e1ac3db809e837be42ee3a9cf451474fa5865',
'thumbnail_width': 640,
'thumbnail_height': 640,
'html': '<div style="max-width: 500px;"><div style="left: 0; width: 100%; height: 0; position: relative; padding-bottom: 100%; padding-top: 80px;"><iframe src="https://open.spotify.com/embed/track/2XsTC2dPbmWREu5RQ1mQC0" style="border: 0; top: 0; left: 0; width: 100%; height: 100%; position: absolute;" allowfullscreen scrolling="no" allow="encrypted-media"></iframe></div></div>',
'cache_age': 86400,
'options': {
'_horizontal': {
'value': false,
'label': 'Slimmer horizontal player'
}
}
}
https://ckeditor.com/ckeditor-4/demo/
{
'id': '9ptZtgd',
'url': 'https://ckeditor.com/ckeditor-4/demo/',
'type': 'rich',
'version': '1.0',
'title': 'CKEditor 4 Demo - The battle-tested WYSIWYG HTML editor',
'provider_name': 'ckeditor',
'description': 'Demo of the battle-tested rich text editor, when you need even more features and legacy compatibility.',
'thumbnail_url': 'https://ckeditor.com/assets/images/og/ogimage-ck4-1ea514a336.png',
'thumbnail_width': 971,
'thumbnail_height': 509,
'cache_age': 86400,
'html': '<div class="iframely-embed"><div class="iframely-responsive" style="padding-bottom: 52.4202%; padding-top: 120px;"><a href="https://ckeditor.com/ckeditor-4/demo/" data-iframely-url="//if-cdn.com/9ptZtgd">CKEditor 4 Demo - The battle-tested WYSIWYG HTML editor</a></div></div><script async src="//if-cdn.com/embed.js" charset="utf-8"></script>'
}
What we can do
- Keep things how they are.
- Use response to create our own content.
- Allow script provided with response to create an iframe. That might be problematic with undo plugin.
- List urls that we support, and don't create widgets when url doesn't match it.
None is optimal, so I'd like have some opinions here.
Optionally we could add a button (It would fit perfectly in balloon toolbar) to convert widgets into links and vice versa.
First response (Spotify) and its response.html
doesn't have a script
tag:
<div style="max-width: 500px;">
<div style="left: 0; width: 100%; height: 0; position: relative; padding-bottom: 100%; padding-top: 80px;">
<iframe src="https://open.spotify.com/embed/track/2XsTC2dPbmWREu5RQ1mQC0" style="border: 0; top: 0; left: 0; width: 100%; height: 100%; position: absolute;" allowfullscreen scrolling="no" allow="encrypted-media"></iframe>
</div>
</div>
which means the final response shape will not be altered by script.
The second one (ckeditor.com
) have a script
tag in response.html
:
<div class="iframely-embed">
<div class="iframely-responsive" style="padding-bottom: 52.4202%; padding-top: 120px;">
<a href="https://ckeditor.com/ckeditor-4/demo/" data-iframely-url="//if-cdn.com/9ptZtgd">CKEditor 4 Demo - The battle-tested WYSIWYG HTML editor</a>
</div>
</div>
<script async src="//if-cdn.com/embed.js" charset="utf-8"></script>
which means it should be changed by an attached script.
Can we use the response.html
to determine if the response should be used directly (no script
tag) or if we should compose our custom HTML (script
tag)?
One more thing, there are some responses containing different script tags. For example, Twitter links have response.html
like:
<blockquote class="twitter-tweet" data-dnt="true" align="center" data-conversation="none">
<p lang="en" dir="ltr">thx! This library is very nice! I will use it to make better products!🤩</p>
— Haruki Tazoe🏍 (@jdkfx)
<a href="https://twitter.com/jdkfx/status/1146678886643617792?ref_src=twsrc%5Etfw">July 4, 2019</a>
</blockquote>
<script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
so they both have "proper" HTML and script tag (which is removed making displayed response not as rich as it could be).
So probably we should determine handling the response based on if response.html
contains iframely custom script - <script async src="//if-cdn.com/embed.js" charset="utf-8"></script>
(could be problematic if the script URL changes)? WDYT @engineering-this? Does it seem doable and reasonable? cc @wwalc
Initially I didn't investigate it carefully enough and took some assumptions.
Img which I thought that is a preview of content is actually just our overlay which prevents interactivity of content in editor.
Urls which response.html contain script, but not iframe: twitter, instagram, imgur
Urls which response.html doesn't contain iframe, but does contain iframe: placekitten, youtube, spotify
We might assume, that we need to create our own content only when script src is "
//if-cdn.com/embed.js`.
This issue is a little bit tricky as there are few different approaches how to solve this. We did some additional brainstorming with @engineering-this and there are few options. However, lets first put some context how omitting scripts for embeds affects content in CKEditor itself vs content created via CKEditor displayed on the frontend layer. It seems to be known behaviour but also sometimes considered as an issue (e.g. #2264).
Since embed scripts are not executed inside CKEditor the editable content looks different than CKEditor output displayed on the frontend (because scripts are executed there):
In editor | Output |
---|---|
![]() |
![]() |
![]() |
![]() |
![]() |
![]() |
This inconsistency might be problematic because content layout will change, however we haven't seen many issues reported relates to this I think 🤔 Let's get to possible solutions then:
Use omit_script=1
iframely option
Starting from omit_script=1
as I will reference to it later. The omit_script
option prevents iframely from attaching script to response.html
. Their docs says:
In that case, Iframely will wrap e.g. Twitter, Instagram, Facebook etc into our <iframe ... > code. For providers that already give an iFrame, Iframely will simply ignore that parameter and will follow your other preferences.
and
When you request &omit_script=1, our own script will not be included with HTML code at all and you need to add it to your site yourself. See below.
But from what I tested, embedding Twitter (no iframe and script
present) or Spotify works the same, however, custom URLS like https://ckeditor.com
works different (better?):
Before:
After:
Pros
- No need to implement anything?
- The script is not included so output HTML will look the same as in editor.
Cons
- It relies on 3rd-party provider configuration (and we should not force users to do that).
- The script is not included so output HTML will look the same as in editor (and not as it would look after the iframely script transforms it), but only for iframely custom script (however the script can be added manually too).
Compose our own content only if response.html
contains //if-cdn.com/embed.js
script
This was the initial idea proposed in https://github.com/ckeditor/ckeditor-dev/issues/2306#issuecomment-412032049. So we use what iframely gives us - title, description, image and then compose nice embed similar to iframley native one.
Pros
- Easy to implement and should work for all cases.
- We have full control over how the content looks inside editor.
Cons
- Still the output is original iframely
response.html
which may be different than our custom implementation. - Iframely format may change making output content even more inconsistent.
- Iframly
//if-cdn.com/embed.js
script path may change which will break the detection mechanism 😱
Attach entire response.html
inside sandbox and copy final HTML as embed
This approach somehow appeared during F2F talks and seemed like a neat idea which will allow having the same content in editor and output data. @engineering-this already tested this approach both with iframe
and shadow DOM sandboxing (both inside and outside of the editor). However, there are few problems we are unable to overcome (problem with detecting final dimension and moment in time when embed script has finished modifying the content).
Pros
- Same content inside and outside the editor consistent with what iframely produces.
- Future-proof for any changes in iframely (like the one causing this issue).
Cons
- We are not able to reasonably fully implement such approach? 😭
To sum up the reasonable and low-cost approach is to go with 2nd solution (Compose our own content only if response.html
contains //if-cdn.com/embed.js
script
), which allows to solve this issue rather swiftly. However, I'm for reporting an issue with embed format inconsistency as a follow-up so we may get back to it and maybe came up with some clever solution.
Related #2264, #3132.