Failing manual test: tests/plugins/widget/manual/block

[sculpt0r]

Type of report

Bug

Provide detailed reproduction steps (if any)

1. Build ckeditor4-preset for full-all
2. Run bender in builder version
3. Open manual test at tests/plugins/widget/manual/block
4. Insert "double column" via "empty button"
5. Place some letter inside, for easier "drag" the entire widget
6. Insert another "double column" inside the current one.
7. Catch and drag the outer widget into the inner one (reference: video)

2021-01-25.12-32-18.mp4

Expected result

What is the expected result of the above steps?
Nothing happened. As the test scenario says: "Should not be possible"

Actual result

What is the actual result of the above steps?
There is some empty insertion below the inner widget.

Other details

• Browser: All
• OS: Windows 8.1 (VM)
• CKEditor version: 4.16.0 (pre-release), introduced in 4.15.0
• Installed CKEditor plugins: presets full-all

[f1ames]

Let's check if it happens in other browser too.

[bunglegrind]

Let's check if it happens in other browser too.

on Win10: Firefox (latest), Chrome (latest) - since 4.15.0 (included)

[bunglegrind]

the regression was introduced in:

9c9d5ac

[bunglegrind]

It looks like that adding:

if ( 'data-cke-widget-drag-handler' in attrs ) {
    element.remove();
}

in one of the two 'toDataFormat' handlers in the widget plugin fixes the issue. Which one? I don't know, I haven't examined the htmldataprocessor accurately. Probably the one with priority 8 (in order to remove the drag handler icon asap).

[Comandeer]

That's a really tricky one.

It seems that the issue is connected with the way we process data to be inserted into nested editable. During toDataFormat widgets in data should be downcasted. However it's not the case, because the data is inserted into nested editable after destroying dropped widget so widget's HTML is treated as "normal" HTML, not widget's one. This triggers adding additional wrapper to the widget during toHtml processing phase and eventually it leads to upcasting widget, but with additional wrapper and drag handlers elements – leftovers from the failed downcasting:

<div tabindex="-1" contenteditable="false" role="region" aria-label="Widget div">
	<div id="w2" data-widget="testdatafilter">
		<div class="foo">
			<p></p>
		</div>
	</div>
	<span style="background:rgba(220,220,220,0.5);background-image:url(img);display:none;">
		<img width="15" title="title" height="15" role="presentation" src="img" />
	</span>
</div>

I see two possible solutions:

1. Do not require widget during downcasting – but it sounds illogical and can lead to weird situations (e.g. trying to downcast widgets that are not defined in the current editor).
2. Change the current data processing flow for nested editables – toDataFormat is used mainly to unescape content. It can be achieved by other means (e.g. exposing unescape logic in the data processor).

I'd go with 2. solution as it seems to have a smaller potential to create weird errors.

There was also another issue: the fact that the parent can be dropped into its child. However this issue is easily resolvable by adding a simple check to drop handler.

[CKEditorBot]

Closed in #4618