Problem/Motivation

Currently the editor user has no feedback if the URL entered to the embed dialog box is valid or not. If the embed object gets an empty response to ::getCode(), we are silently creating the wrapper with an empty content. The problem with this is that it's impossible to the end user to edit the wrapper back inside the CKEditor preview, and change the URL to a new one. The only solution is to view the source, and delete / modify the code from there.

Proposed resolution

if $info->getCode() returns empty, put an informative string inside the wrapper, allowing users to click on it and modify / delete it without having to go into the source.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 url_embed-non_embeddable_urls-2761187-11.patch745 bytesloze
#9 urlembed-non-embeddable-urls-2761187-9-d8.patch823 bytesnavneet0693
#2 improve-handling-of-non-embeddable-URLs-2761187-2.patch613 bytesmarcoscano

Issue fork url_embed-2761187

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

marcoscano created an issue. See original summary.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented
Assigned: marcoscano » Unassigned
Status: Active » Needs review
Related issues: +#2551167: Embed dialog missing validation if the URL is embeddable or not
StatusFileSize
new improve-handling-of-non-embeddable-URLs-2761187-2.patch613 bytes

Something like this would be enough to improve the user experience with these URLs.

cassien’s picture

cassien commented

That little hack saved my day.
I was pretty frustrated about the urls non embedable wich were not displayed.
I just replaced the dialog to simply show the url with a link

$url_output = !empty($url_output) ? $info->getCode() : '<a href="'.$url.'">'.$url.'</a>';

socialnicheguru’s picture

socialnicheguru commented

Conflicts with https://www.drupal.org/project/url_embed/issues/2871744

jaapjan’s picture

jaapjan commented

What is the status of this exactly?

URL Embed is still in a 2-year old alpha1 phase. It would be nice to move this forward.

Although the patch from comment 2 works, I propose to just display the old URL instead of the text.
E.g. a fallback like this would be sufficient in my case:

use Drupal\Core\Link;
use Drupal\Core\Url;

if ($url_output === NULL || $url_output === '') {
  $url_output = Link::fromTextAndUrl($url, Url::fromUri($url))->toString();
}
justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented
Related issues: +#2864302: Add validation to ensure URL is embeddable

This is solved nicely in #2864302: Add validation to ensure URL is embeddable, which has an RTBC patch. Can this be closed as a duplicate?

justcaldwell’s picture

justcaldwell
he/him
Primary language English
Location Austin, Texas
commented
Status: Needs review » Closed (duplicate)
navneet0693’s picture

navneet0693 commented
Status: Closed (duplicate) » Active

That issue is just adding a validation for workaround for avoiding users to not enter the URL which non-embeddable and that does not solves the problem. The problem is bigger than that.

navneet0693’s picture

navneet0693 commented
Status: Active » Needs review
StatusFileSize
new urlembed-non-embeddable-urls-2761187-9-d8.patch823 bytes
Yuri’s picture

Yuri commented

(using D9.0.3) the patch #9 above gives:

The website encountered an unexpected error. Please try again later.
Error: Class 'Drupal\url_embed\Plugin\Filter\Link' not found in Drupal\url_embed\Plugin\Filter\UrlEmbedFilter->process() (line 83 of modules/contrib/url_embed/src/Plugin/Filter/UrlEmbedFilter.php).
Drupal\url_embed\Plugin\Filter\UrlEmbedFilter->process('Option:
)
loze’s picture

loze commented
StatusFileSize
new url_embed-non_embeddable_urls-2761187-11.patch745 bytes

The patch in #9 is missing the use statements for Link and Url mentioned in #5
However, I don't think this filter should convert the url to a link. If you want it displayed as a clickable link you can run the core "convert urls to links" filter after this one.

Here is a patch.

strykaizer made their first commit to this issue’s fork.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented

Pushed branch fixes this issue by not altering (nor wrapping) urls with data-embed-url.
This allows to use core "Convert URLs into links" filter triggering after this filter without issues.

strykaizer opened merge request !25

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented
Version: 8.x-1.x-dev » 3.x-dev

mark_fullmer’s picture

mark_fullmer
he/him
Primary language English
Location Tucson
commented
Status: Needs review » Fixed

The code change in the MR makes perfect sense, and I think it also reflects how the code should have been implemented in the first place, rather than an affordance for an edge case. In that sense, I agree with the comment in #8, arguing for this in favor of #2864302: Add validation to ensure URL is embeddable . Merging. Thanks, everyone!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.