Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#13640 closed Bug (fixed)

[IE] Dropping a widget outside body is not handled correctly

Reported by: Piotrek Koszuliński Owned by: Szymon Cofalik
Priority: Normal Milestone: CKEditor 4.5.4
Component: General Version: 4.5.0
Keywords: Cc:

Description

Steps to reproduce

  1. Open http://tests.ckeditor.dev:1030/tests/plugins/clipboard/manual/draganddrop
  2. Drag the placeholder to the left margin of the content (outside body, but inside an iframe)

Expected result

Nothing bad should happen (widget may stay selected or a selection can be placed before/after it.

Actual result

Something weird happens with the widget. The DOM structure is preserved but it's not a real widget, so e.g. if you switch to source mode you can see that it wasn't downcasted. The same with clicking on it.

Other details (browser, OS, CKEditor version, installed plugins)

  • Browser: IE11
  • I wasn't able to reproduce it with image2, so it's not super general for widgets. I believe that a widget must have a textual content inside.
  • Before #13465 and #13453 a different error was thrown, so it wasn't possible to very this bug. I set version to 4.5.0 as I think it may be related to the new DnD system.

Change History (8)

comment:1 Changed 3 years ago by Marek Lewandowski

Another TC from IE9:

  1. Open tests/tickets/13465/1(http://tests.ckeditor.dev:1030/tests/tickets/13465/1#child)
  2. focus placeholder
  3. drag and drop the placeholder "before itself"
  4. check source code

Expected:

<p>[[foobar]]</p>

Actual:

<p><span contenteditable="false" tabindex="-1"><span data-widget="placeholder">[[foobar]]</span><span style="background:url(&quot;http://192.168.20.115:1030/apps/ckeditor/plugins/widget/images/handle.png&quot;) rgba(220, 220, 220, 0.5); display:block; left:0px; top:-12px"><img draggable="true" src="data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw==" style="height:15px; width:15px" title="Click and drag to move" /></span></span></p>

Last edited 3 years ago by Marek Lewandowski (previous) (diff)

comment:2 Changed 3 years ago by Marek Lewandowski

Summary: [IE11] Dropping a widget outside body is not handled correctly[IE] Dropping a widget outside body is not handled correctly

comment:3 Changed 3 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.4
Status: newconfirmed

comment:4 Changed 3 years ago by Szymon Cofalik

Owner: set to Szymon Cofalik
Status: confirmedassigned

comment:5 Changed 3 years ago by Szymon Cofalik

The problem is introduced by #13453 where we decided, that if the drop range is inside drag range, we should "simulate cancelling" drop algorithm so we won't fire unnecessary events. It appeared, that those events are necessarry after all.

During widgets D&D we destroy the widget in one of events. In normal case scenario, the widget is then re-created in paste event (after firePasteEvents method is fired in clipboard.internalDrop). However when we "cancel" paste event, we won't get to re-create the widget.

It worked on non-IE browsers because dropInsideDragRange was incorrectly declared (used == instead of &). This was also corrected.

There are two ways we can go now: we either stick with original solution and "cancel" the event, but recreate widgets anyway (branch:t/13640) or abandon event "cancelling" and just put drop range right before and continue with normal algorithm execution (branch:t/13640b).

We also should re-test #13453, that's why I changed it's test description (added 4.5.4 tag). I haven't added any new tests because tests for this already exist.

Last edited 3 years ago by Szymon Cofalik (previous) (diff)

comment:6 Changed 3 years ago by Szymon Cofalik

Status: assignedreview

comment:7 Changed 3 years ago by Piotr Jasiun

Resolution: fixed
Status: reviewclosed

First solution is incorrect: clipboard plugin should know nothing about widgets. It should not access editor.widgets.checkWidget, because if widget plugin will be not loaded editor.widgets will be undefined.

But the second solution seems to be good. In general paste event should not be prevented on that stage, after the drop event is not canceled, so pasting the content before drag position looks like a proper way.

And, by the way, I admire your strategy. You know that usually the first solution get R-, so put two solutions at once. :)

Closed with git:b98d8b9.

Last edited 3 years ago by Piotrek Koszuliński (previous) (diff)

comment:8 Changed 3 years ago by Szymon Cofalik

It makes my stats looks better :>

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy