ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Ck/12467 inser content does not insert markers

Open DawidKossowski opened this issue 3 years ago • 2 comments

Suggested merge commit message (convention)

Other (engine): The functionality of the insertContent method has been extended by handling markers. Closes #12467.


Additional information

This improvement has been implemented to allow Import from Word plugin to work correctly.

DawidKossowski avatar Sep 20 '22 10:09 DawidKossowski

@scofalik all done (I hope so...). You can review it again.

DawidKossowski avatar Sep 22 '22 13:09 DawidKossowski

I forgot to mention that the issue with merging $marker element is still existing. We need to figure out how to fix it.

DawidKossowski avatar Sep 22 '22 13:09 DawidKossowski

@scofalik it's ready to review. However, there are still issues that we discussed and will be fixed in the following tasks:

  1. We use affectedRange to calculate starting and ending markers' positions. It causes that sometimes markers are created in the wrong place (like inserting imageBlock inside paragraph):
// <paragraph>Fo[]o</paragraph>
// {<imageBlock></imageBlock>}
//
// <paragraph>Fo{</paragraph><imageBlock></imageBlock><paragraph>}o</paragraph>
  1. Sometimes, when we set the custom selection the resulting insertion is wrong (it's not connected to markers):
setData( model, '<imageBlock></imageBlock>[]<imageBlock></imageBlock>' );

insertHelper( '<imageBlock></imageBlock>', null, [ 1 ] );

expect( getData( model ) ).to.equal( '[<imageBlock></imageBlock>]<imageBlock></imageBlock><imageBlock></imageBlock>' );

DawidKossowski avatar Sep 26 '22 08:09 DawidKossowski

Regarding your previous post:

  1. Please create an issue to improve current solution. Title: "insertContent() should not depend on affected range when inserting markers".
  2. I think this happens because in setData() (or rather: just after) the selection is autofixed to [<imageBlock></imageBlock>]<imageBlock></imageBlock>. After you insert between the paragraphs, it does not affect document selection in any way. Since you passed the selection by hand to insertContent(), insertContent() does not change the document selection either. So, everything looks good here, after all.

scofalik avatar Sep 26 '22 11:09 scofalik

What about a case where marker starts in filtered content but ends in non-filtered content (and vice versa)?

scofalik avatar Sep 26 '22 11:09 scofalik

@scofalik you can take a look at it again.

DawidKossowski avatar Sep 27 '22 13:09 DawidKossowski

@arkflpc Could you look at it from TS perspective?

scofalik avatar Sep 27 '22 14:09 scofalik