[Blink, FF] Block Elements removed when we copy & paste

[Satya Minnekanti]

Steps to reproduce

1. Open a sample with ACF disabled.
2. Create a Block level element ( ex: H1)
3. select the Block lelvel elemnt( ex: H1) with mouse or use Shift + Home
4. Ctrl + C to copy
5. Press Enter to go to a new paragraph
6. Ctrl + V to paste

Expected result

Block Level element(ex: H1) pasted properly with all attributes(if any applied)

Actual result

Pasted as plain text with out Block level element and it's attributes

This is a regression from 4.5.x & it's HIGH priority defect for us & we have customer PMR

[Marek Lewandowski]

cc

[Jakub Ś]

I can't confirm that this is ACF dependent (got same results with ACF on/off) but I can definitely confirm this issue happening in all browsers.

[Satya Minnekanti]

Replying to j.swiderski:

I can't confirm that this is ACF dependent (got same results with ACF on/off) but I can definitely confirm this issue in Firefox, Blink and Webkit (IE is free of this bug - didn't check EDGE).

@j.swiderski Thanks for reply. I could reproduce the issue in IE11 & EDGE as well on nightly build. ​http://nightly.ckeditor.com/16-01-29-07-07/full/samples/

[Jakub Ś]

@satya you are right. There is one case which works and it happens I have tested it - keyboard only.

So set H1 in format dropdown, type, press Home, press Shift+End, Ctrl+C, End, Enter, Ctrl+V - that way and actually only that way it works. I have modified my description.

[Jakub Ś]

#14391 was marked as duplicate.

Please check TC mentioned in that ticket. If you have any doubts if this behaviour should also be applied to paragraphs we can discuss it with m.lewandowski and IBM

[kkrzton]

It is a regression from 4.5.x, it worked fine in 4.4.8 (in all browsers but IE). Since 4.5.x custom copy handling was introduced (for all browsers but IE). In earlier versions copying (copy/cut) was handling natively.

Custom copy handling introduced in 4.5.0 does not support IE because of some limitations in ClipboardAPI in IE implementation. It means copy handling in IE hasn't change across previously mentioned versions, copying headers also doesn't work in versions below 4.5.0 in IE. Due to this differences in copy handling, we decided to split this ticket into two, one is for fixing this issue in other browsers than IE and the other one is to support this in IE. It will help us to work on this issues separately and provide solution faster.

IE issue #14397.

[kkrzton]

Replying to j.swiderski:

#14391 was marked as duplicate.

Please check TC mentioned in that ticket. If you have any doubts if this behaviour should also be applied to paragraphs we can discuss it with m.lewandowski and IBM

We also reopened #14391 due to differences in handling paragraphs versus other block elements ​#14391#comment:4.

[kkrzton]

Changes in ​t/14358.

[Marek Lewandowski]

editor.getSelectedHtml methods seems to be the right place to implement it, however current solution has couple of flaws:

• When one copies only part of header text, header markup is applied. Header (block styling) should be copied only when whole block's content is selected.

Consider following example:

<h3>foo [bar baz] bom</h3>

Editor should copy only bar baz text. Ofc it might contain inline styles, that's fine.

Block style should only be copied when the element is selected entirely, like so:

<h3>[foo bar baz bom]</h3>

• It only covers headers.

In fact we should care about any block styles.

List handling will be problematic, so we can extract it to a separate ticket.

Having this done it will fix #14391.

• It should apply to as many blocks in path as possible.

Actually it should also be possible to include multi level blocks.

Consider blockquote p or ul li p.

Here is a TC:

<blockquote>
	<p>[foo bar baz]</p>
</blockquote>

should be expanded to contain the block:

[<blockquote>
	<p>foo bar baz</p>
</blockquote>]

Note "Expansion" of copied path should also be stopped by [contenteditable=false] elements (e.g. widget wrappers).

[kkrzton]

When one copies only part of header text, header markup is applied. Header (block styling) should be copied only when whole block's content is selected.

Yes, it works this way and it was made intentionally. This bug was introduced when the custom copy/paste was introduced (4.5.0). Before, it was handled by native copy/paste browser implementation. In Chrome and FF native copy/paste works this way, so copying part of header also creates header element. I'm not saying that this is a behaviour we would like to implement only that this solution was based on how it worked before.

In fact we should care about any block styles.

Yes, I agree we should introduce more general solution to handle as many block elements as possible which should also fix #14391 (or any related future issues). Partial coping/pasting as mentioned above may not make sense for headers but considering #14391 I think it make sense and is rather intuitive.

One more thing worth mentioning is that while native copy/paste works for headers as described above it does not work for e.g. paragraphs (so #14391 is more like a new feature than bug - it is about copying element attributes not only element). It shouldn't matter much for general solution but it should be tested solidly.

[kkrzton]

We agreed with @m.lewandowski that:

• Header (block styling) should be copied only when whole block's content is selected. This applies to all block elements. All block elements attributes except id also should be copied.
• We should care about any block styles/elements so we will introduce more general solution.
• It should apply to as many blocks in path as possible. "Expansion" of copied path should also be stopped by [contenteditable=false] elements (e.g. widget wrappers) and table elements (like td, th).

[kkrzton]

I have some doubts about removing id while copying whole block. Copying is not an issue and we don't know what user is going to do with copied content. The solution could be to remove id while pasting but then we should only remove it if the element with given id already exists in editor.

I think it is broader issue, not only connected to this ticket but to the whole pasting logic. Probably it should be handled in separate ticket as bug / feature request.

@m.lewandowski what do you think?

[Marek Lewandowski]

@k.krzton Completely agreed, it's logical. Now I wonder why we didn't think about it that way from the very beginning. :)

[kkrzton]

@m.lewandowski Created new feature request #14455 for issue mentioned in #comment:14.

[kkrzton]

Changes in ​t/14358.

Introduced mechanism for recognizing if entire block element is selected. If there is, the outermost entirely selected block will be copied which results in preserving styling while pasting such elements. As mentioned in above comment

"Expansion" of copied path should also be stopped by [contenteditable=false] elements (e.g. widget wrappers) and table elements (like td, th).

non-editable elements, table elements (td, th and also caption) and lists elements stops path expansion. Copying whole list while one item is entirely selected is under consideration and will be handled separately, but still all formatting inside lists will be copied correctly.

This solution also fixes #14391.

There is one difference in Blink vs. other browsers in handling empty blocks (reported here #14501) so for case mentioned in this ticket, it will work differently. While CKEditor keeps clean html and such empty elements are not a common case it may be handled in future (basically by fixing this issue #14501).

[Marek Lewandowski]

Rebased the branch to the latest master with a minor adjustment. Still under the review though.

[Jakub Ś]

It turns out we have quite a few issues with pasting and they all may be related as they all can be reproduced from 4.5.0:
Styles Stripping: #13860, #13926, #14250, #14358
Styles stripping on Chrome in various test cases: #14921, #14593, #13753, #13751, #16454
ForcePasteAsPlainText pastes formatted HTML with Ctrl+V: #13969
Weird event pasting relation (not sure): #13763
White spaces not preserved: #14614
Problem with pasting in LibreOffice: #14622 (we do not support it but I have added this issue because general solution might also bring back old behaviour).

[Marek Lewandowski]

Moving tickets to the next milestone.

[Marek Lewandowski]

Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.

[Marek Lewandowski]

We won't be able to deliver it safely in 4.7.0. Moving to backlog.