ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.6k stars 3.7k forks source link

Incorrect selection behavior when inserting SVGs inside of an inline widget #14388

Open mabryl opened 1 year ago

mabryl commented 1 year ago

πŸ“ Provide detailed reproduction steps (if any)

  1. Create an inline widget plugin based on the Implementing an inline widget guide
  2. Make the widget insert an SVG, for example, by using this code snippet:
    const innerText = viewWriter.createRawElement( 'i', { id: 'foo-1234' }, function( domElement ) {
    domElement.innerHTML = `
    <svg fill="#000000" width="50px" height="50px" viewBox="-6.5 0 32 32" version="1.1" xmlns="http://www.w3.org/2000/svg">
      <path d="M9.281 9.688h2.406v2.406h-2.406v-2.406zM9.813 21.969l2.188 1.031-2.906 6.125-1.906-0.906z"></path>
    </svg>
    `;
    } );
  3. Build the editor with the plugin included
  4. Try placing the selection after the inserted SVG

βœ”οΈ Expected result

The caret should appear to the right of the SVG, as is the case when inserting text via this method:

https://github.com/ckeditor/ckeditor5/assets/72079603/7a9df9bc-91d1-4c96-a971-fa6af528a273

❌ Actual result

You can't place the caret after the SVG. This example is from Google Chrome:

https://github.com/ckeditor/ckeditor5/assets/72079603/ac32dce9-ab20-4536-baba-79a71dad8cc6

πŸ“ƒ Other details


If you'd like to see this fixed sooner, add a πŸ‘ reaction to this post.

mabryl commented 1 year ago

As mentioned, this bug is different depending on the browser that you use. In Safari, you can place the selection after the widget but the caret is huge:

https://github.com/ckeditor/ckeditor5/assets/72079603/1dbbc621-c426-44cb-b40f-f8b64c3a5f87

mabryl commented 1 year ago

And Firefox acts similarly to Chrome:

https://github.com/ckeditor/ckeditor5/assets/72079603/bcf02a7d-cf72-49fb-a1e8-d6db3e5a07e8

TerryCaliendo-WealthCounsel commented 1 year ago

My temporary work-around is to add a tiny space after the svg.

const innerText = viewWriter.createRawElement( 'i', { id: 'foo-1234' }, function( domElement ) {
  domElement.innerHTML = `
    <svg fill="#000000" width="50px" height="50px" viewBox="-6.5 0 32 32" version="1.1" xmlns="http://www.w3.org/2000/svg">
      <path d="M9.281 9.688h2.406v2.406h-2.406v-2.406zM9.813 21.969l2.188 1.031-2.906 6.125-1.906-0.906z"></path>
    </svg>
    <span style="font-size: 1px">&nbsp</span>
  `;
} );

I've only tested it in Chrome and in my custom project. However, this work-around does seem to have an issues where the svg/widget will suddenly disappear after some actions in the editor, but I haven't yet found a way to consistently re-create the disappearance.

Witoso commented 1 year ago

@mabryl could you share more code? It's unclear whether 1) innerText is wrapped in some other element 2) which element is wrapped into toWidget method.

TerryCaliendo-WealthCounsel commented 1 year ago

I created a demo of how I discovered the issue in our custom configuration here:

https://www.dropbox.com/s/un3gw9cljb292k3/2023-06-12%2010-17-37.mkv?dl=0

mabryl commented 1 year ago

@Witoso sharing my project in a .zip below. You can just open the index.html file and it should work. Choosing any Placeholder from the dropdown menu should insert the SVG.

By the way, I'm not sure whether it's an issue limited to only SVGs (I'm guessing it's not) but for example <img> tags inserted by using this method worked correctly and didn't break the selection.

@TerryCaliendo-WealthCounsel many thanks for sharing the video here.

inline_svg.zip

Witoso commented 1 year ago

It may be related to some extent to us not supporting svg's (we treat it as an unsafe element AFAIK). Although it shouldn't matter for RawElements. We need to investigate it. Interesting that the keyboard navigation works fine.

Witoso commented 1 year ago

Long story short, browsers would like to place a caret in a text-like element, and the line ends without such. (similar to #12267). The scope of the fix would be to place an inline filler, the same one we place at the beginning of the line.

@TerryCaliendo-WealthCounsel for the workaround, instead of nbsp, try to use zws (​&#8203;/​&ZeroWidthSpace;) for better behavior. I'm a bit worried about the situation where the svg/widget will suddenly disappear but the inserted space should not influence the editor in this way. Let us know when you observe this situation again.

We will add this to our bug-fixing queue.

TerryCaliendo-WealthCounsel commented 1 year ago

@Witoso - thanks for the update. I will try the Zero Width Space. The code is in our QE (Quality Engineer) process and I'm waiting to see if the have the "disappearing" issue. I saw it twice but haven't been able to recreate since.

tyleregeto commented 1 year ago

I am seeing this same behaviour with RawElement, and non SVG content. I've tried a variety of tags types that result in the same behaviour, such as span and a tags. Keyboard navigation works fine, but clicking always moves the cursor. If there is any text content after the raw element, there are no issues.

In my case, I was able to reproduce this with this minimal RawElement usage:

return writer.createRawElement('span', {}, (domElement) => {
    domElement.innerText = 'RawElement Content';
});

Video attached for reference.

https://github.com/ckeditor/ckeditor5/assets/1633730/7ab6e998-e22f-4122-a5bc-67ee28ddf857

This video was taken in Chrome.

Witoso commented 1 year ago

I am seeing this same behaviour with RawElement

Citing the docs:

The raw elements work as data containers ("wrappers", "sandboxes") but their children are not managed or even recognized by the editor.

The best way is to pack the raw element into a widget in the editing view, and the behavior should work.

gauravmanerkar commented 10 months ago

I am seeing this same behaviour with RawElement

Citing the docs:

The raw elements work as data containers ("wrappers", "sandboxes") but their children are not managed or even recognized by the editor.

The best way is to pack the raw element into a widget in the editing view, and the behavior should work.

Raw element doesn't support toWidget

Witoso commented 10 months ago

@gauravmanerkar just create a container element and wrap the raw one in it. There's an example of such thing in our docs.