Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inserting images by pasting URL directly into editor #8236

Closed
pkwasnik opened this issue Oct 9, 2020 · 7 comments · Fixed by #8372
Closed

Inserting images by pasting URL directly into editor #8236

pkwasnik opened this issue Oct 9, 2020 · 7 comments · Fixed by #8372
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. intro Good first ticket. package:image squad:core Issue to be handled by the Core team. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@pkwasnik
Copy link
Contributor

pkwasnik commented Oct 9, 2020

📝 Provide a description of the new feature

It could be usefull to have possibility to insert image just by pasting URL to the editor like it works in media-embed.


If you'd like to see this feature implemented, add a 👍 reaction to this post.

@pkwasnik pkwasnik added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:image package:media-embed domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. labels Oct 9, 2020
@pkwasnik pkwasnik changed the title Support for inserting images Inserting images by pasting URL directly into editor Oct 9, 2020
@jodator jodator added the intro Good first ticket. label Oct 12, 2020
@Reinmar Reinmar removed the squad:core Issue to be handled by the Core team. label Oct 12, 2020
@Reinmar Reinmar added this to the backlog milestone Oct 12, 2020
@oleq
Copy link
Member

oleq commented Oct 12, 2020

This sounds like a cool UX enhancement. There are some questions waiting to be answered, though:

  • The feature must be configurable because we cannot rely on the "file" extension in the URL (the URL can be anything). What the configuration would look like? I assume here there would be no default configuration.
  • CSP will certainly be a problem. Can we detect when an URL is blocked by CSP? If so, how to let the user know what happened and what to do next?
  • Do we want the image to be "simply embedded as it is" or re-uploaded by the upload adapter ("absorbed" by the webpage that uses CKEditor 5)? I think both strategies are valid use-cases.

@jodator
Copy link
Contributor

jodator commented Oct 12, 2020

CSP will certainly be a problem. Can we detect when an URL is blocked by CSP?

I don't think we do this for the image inserted by URL input? (there's a validation issue to be resolved as well #7917 ).

Do we want the image to be "simply embedded as it is"

I'd go with duplicating whatever is happening using the insert image UI. At least in the MVP.

The feature must be configurable because we cannot rely on the "file" extension in the URL (the URL can be anything)

That's tricky, but having some simple list of extensions would suite most of the cases and it shouldn't do more than that. This should handle valid URL resource: http(s)://(www.)example.com/path/to/resource.ext?query=params&maybe=too. For everything else, the UI would handle that.

@Reinmar
Copy link
Member

Reinmar commented Oct 12, 2020

Oh, I've been thinking about inserting === uploading. That request appeared in the past and makes total sense as you shouldn't usually reference images from other websites.

So, the upload via URL thing would be awesome, but AFAIR, there was a CSP problem. A backend solution would be a solution, though, but it makes this a bigger topic.

As for simple inserting, it's much easier. I can imagine doing that by a file extension-driven heuristic. So, I agree with:

That's tricky, but having some simple list of extensions would suite most of the cases and it shouldn't do more than that. This should handle valid URL resource: http(s)://(www.)example.com/path/to/resource.ext?query=params&maybe=too. For everything else, the UI would handle that.

@Reinmar Reinmar added the squad:core Issue to be handled by the Core team. label Oct 12, 2020
@pkwasnik
Copy link
Contributor Author

We already have adding images using URL implemented, so for most cases it could work the same. But right, when pasting to the editor there is a problem with recognizing if it is an image.

We could first filter links using something like @jodator propose. Don't know how our engine works, but I suppose that when it will try to show the resource it will know if it is something it could render. Alternatively we could try to read some meta data from the resource.

@oleq
Copy link
Member

oleq commented Oct 12, 2020

Oh, I've been thinking about inserting === uploading. That request appeared in the past and makes total sense as you shouldn't usually reference images from other websites.

Yep, inserting is way easier and CSP is not a big deal here as Jodi pointed out:

I don't think we do this for the image inserted by URL input? (there's a validation issue to be resolved as well #7917 ).

Don't know how our engine works, but I suppose that when it will try to show the resource it will know if it is something it could render.

The engine has very little to do with this, it does not actively recognize the content nor analyze it. The image URL discovery must be on the Image feature side and can be done in 3 ways:

  1. Via RegExp provided in editor configuration.
  2. Via pre-loading the URL off the editing root as <img src="{URL}"/> (in some hidden container) and checking if it really was an image and only then inserting it into the content as such. 

Alternatively we could try to read some meta data from the resource.

This is the third way but we may hit CSP here. AFAIR (but I could be wrong) there's a difference in CSP when you do <img src="{URL}" /> and when you do an XHR to the same URL.

@Reinmar
Copy link
Member

Reinmar commented Oct 12, 2020

2. Via pre-loading the URL off the editing root as <img src="{URL}"/> (in some hidden container) and checking if it really was an image and only then inserting it into the content as such.

Privacy is one concern. We could make this opt-in and mention in the docs this concern.

However, for starter, we could go with the regexp only and wait for the feedback.

To be considered: There's already a logic inside media embed that enhances the link on paste. It's a bit more complex than it sounds because it first let the editor render that link so the user sees it and then replaces it with the media after a short timeout.

Name? AutoImage? :D 

In the first stage, let's not add this to any build.

@pkwasnik pkwasnik self-assigned this Oct 21, 2020
@jodator
Copy link
Contributor

jodator commented Oct 26, 2020

In order to have this feature implement:

  • New AutoImage plugin based on AutoMediaEmbed. Duplication at this stage is OK.
  • Set of automatic tests for AutoImage.
  • Manual test for AutoImage.
  • Follow-up ticket for de-duplicating AutoImageand AutoMediaEmbed.
  • New section in the image feature guide (just befor "installation") for "Automatic image insert on paste" based on auto-media entry. However, since this feature is not enabled by default in any build we need a demo as well. I think that we could provide one image URL to paste similarly to media embed demo.
  • Update info on the image API entry page.
  • After PR is merged, notify the docs team to re-review.

@Reinmar Reinmar modified the milestones: backlog, iteration 38 Oct 26, 2020
jodator added a commit that referenced this issue Nov 4, 2020
Feature (image): Inserting images by pasting URL to an image directly into the editor. Closes #8236.

MINOR BREAKING CHANGE (image): The `insertImage()` Image's utility function parameters have changed. The removed `writer` instance is no longer needed. Additionaly, you can specify `insertPosition` as an optional parameter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. intro Good first ticket. package:image squad:core Issue to be handled by the Core team. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants