Opened 7 years ago

Last modified 6 years ago

#11460 closed New Feature

Handling dropping content in editor — at Version 12

Reported by: Piotrek Koszuliński Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version:
Keywords: Cc:

Change History (22)

comment:1 Changed 7 years ago by Piotrek Koszuliński

Description: modified (diff)
Status: newconfirmed

comment:2 Changed 7 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

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

During today’s discussion with PJ we raised a number of topics which need further discussion or actions or just should be emphasised.

  1. Remark: Paste event may contain DataTransfer object and files list similarly to drop event. Even if it doesn’t contain them currently, it may in the future and we should take that into consideration. This makes both events very similar.
  1. Task: We need full research on current implementations of drop and paste events

Full table of what data (including types) are carried for what type of insertions (both - dropping and pasting) on which browsers is needed. It will help to make decisions regarding API and testing. It may become a compatibility table after the job.

Scenarios (dropping/pasting):

  • HTML content,
  • HTML content with images from outside of editable area,
  • one image from system file browser,
  • many images from system file browser,
  • part of image (data) copied (and dropped if that’s possible) e.g. from image editor (e.g. MS Paint),
  • content of MS Word document with images,
  • text file from system file browser,
  • content of text file.

Focus should be put on what data types are available in drop/paste event and what files are available in files property. Sample used for testing should be attached in this ticket.

  1. Task: We need a short excerpt from both specifications (clipboard and dnd). In previous point I mentioned a compatibility table which will clarify how the situation looks now. But it may change in the future, so specs should be checked.
  1. Question: do we need “drop file here” area? File should be dropped under the cursor, just like inline widget is dropped. It works this way when I drop image into TextEdit - I see a caret and image is inserted at that position. It works differently in Gmail, but image is an attachment there, so it’s a different case. MS Word should be checked.
  1. Question: isn’t my proposal for drop event being forwarded to paste ridiculous? As I mentioned in first point paste is very similar to drop. This means that both should be handled similarly, but definitely not that drop should fire paste. For example, pasting file is (or may be - see point 3.) the same as dropping it - there may be no HTML representation available, so transformation may have to be performed - the same as for file drop. Would we transform files twice or would we do it only during paste event? Both options are hard to justify, so let’s start from beginning.

We need a single event for data insertion - that’s definitely a truth. But our mistake was to accept paste as it. Paste is a paste, drop is a drop - they are similar, but different actions. We need a new event to handle them, therefore events chain should rather look like that:

  • paste -> afterPaste -> insertData
  • drop -> (afterDrop) -> insertData

Shared transformations would be done on insertData and insertData would also finalise insertion by calling editor.insertXXX. Action specific transformations may still be done specifically for past or drop. For example, on some browser we’ll still use pastebin, so we’ll need “HTMLified text normalisation” - this will be done on paste only. However, “plain text to HTML transformation” which will cover pasting and dropping plain text may be done in insertData unless HTML content is already available in event’s data.

The downside is that such changes will break backward compatibility at some point. It may turn out that we’ll be able to keep most of current paste event’s behaviour, but some things will change definitely. I don’t see other option though, because trying to keep current code intact will result in a ridiculous solution. So - good design is first, but backward compatibility is important and should be kept where possible.

Another downside is that we may need to touch a lot of clipboard code now. Theoretically, my idea could be implemented without touching paste event at all (the only difference is that afterPaste has to fire insertData), but making it compatible with drop event may require some modifications, especially in order to keep good code organisation.

comment:4 in reply to:  3 Changed 7 years ago by Piotr Jasiun

Replying to Reinmar:

During today’s discussion with PJ we raised a number of topics which need further discussion or actions or just should be emphasised.

  1. Remark: Paste event may contain DataTransfer object and files list similarly to drop event. Even if it doesn’t contain them currently, it may in the future and we should take that into consideration. This makes both events very similar.

That's a good point.

  1. Task: We need full research on current implementations of drop and paste events

Full table of what data (including types) are carried for what type of insertions (both - dropping and pasting) on which browsers is needed. It will help to make decisions regarding API and testing. It may become a compatibility table after the job.

Scenarios (dropping/pasting):

  • HTML content,
  • HTML content with images from outside of editable area,
  • one image from system file browser,
  • many images from system file browser,
  • part of image (data) copied (and dropped if that’s possible) e.g. from image editor (e.g. MS Paint),
  • content of MS Word document with images,
  • text file from system file browser,
  • content of text file.

Research is avaible in #11526.

Focus should be put on what data types are available in drop/paste event and what files are available in files property. Sample used for testing should be attached in this ticket.

  1. Task: We need a short excerpt from both specifications (clipboard and dnd). In previous point I mentioned a compatibility table which will clarify how the situation looks now. But it may change in the future, so specs should be checked.

Well... the problem now is that specification is far from reality. Event in introduction they are writing about attributes and solutions not supported in some or any browser (like dropzone or custom data type). So probably one day in the future browsers will be closer to the specification but it is far future. The main problem is (of course) IE. Anyway W3C APIs are described in #11526.

  1. Question: do we need “drop file here” area? File should be dropped under the cursor, just like inline widget is dropped. It works this way when I drop image into TextEdit - I see a caret and image is inserted at that position. It works differently in Gmail, but image is an attachment there, so it’s a different case. MS Word should be checked.

In Gmail if you drop image it appear inside a content, not as an attachment (we tested it with Linux, Windows and Mac). If you D&D file in different type then it goes to the attachment. In MS Word it works like text D&D. Anyway this is more UX think so I will move this discussion to mail and involve Anna and Sebastian into discussion.

From the technical point of view I can not get files details until file is dropped. In dropover event I can get only information that some "Files" are over droparea, so I cannot show “drop file here” information only for supported file types. What I can is to change the message just after user drop files from "drop file here" to "this file type is not supported" and hide droparea after some seconds.

  1. Question: isn’t my proposal for drop event being forwarded to paste ridiculous? As I mentioned in first point paste is very similar to drop. This means that both should be handled similarly, but definitely not that drop should fire paste. For example, pasting file is (or may be - see point 3.) the same as dropping it - there may be no HTML representation available, so transformation may have to be performed - the same as for file drop. Would we transform files twice or would we do it only during paste event? Both options are hard to justify, so let’s start from beginning.

We need a single event for data insertion - that’s definitely a truth. But our mistake was to accept paste as it. Paste is a paste, drop is a drop - they are similar, but different actions. We need a new event to handle them, therefore events chain should rather look like that:

  • paste -> afterPaste -> insertData
  • drop -> (afterDrop) -> insertData

Shared transformations would be done on insertData and insertData would also finalise insertion by calling editor.insertXXX. Action specific transformations may still be done specifically for past or drop. For example, on some browser we’ll still use pastebin, so we’ll need “HTMLified text normalisation” - this will be done on paste only. However, “plain text to HTML transformation” which will cover pasting and dropping plain text may be done in insertData unless HTML content is already available in event’s data.

The downside is that such changes will break backward compatibility at some point. It may turn out that we’ll be able to keep most of current paste event’s behaviour, but some things will change definitely. I don’t see other option though, because trying to keep current code intact will result in a ridiculous solution. So - good design is first, but backward compatibility is important and should be kept where possible.

Another downside is that we may need to touch a lot of clipboard code now. Theoretically, my idea could be implemented without touching paste event at all (the only difference is that afterPaste has to fire insertData), but making it compatible with drop event may require some modifications, especially in order to keep good code organisation.

The biggest problem, in my opinion, is that we have two different data structures now.

  • on the one hand we have our old paste event which has only one dataValue in one type (html/text/auto),
  • on the other we have rich DataTransfer which can contain multiple types of data and files in the same time.

And there are two thinks we can do with if this:

  • create some addition step of data transformation when the DataTransfer will be transformed to the html (ex. dropped image file will be read and change to img element) and such html will be pushed to the current paste event,
  • modify current paste event so it contains all of the date (files and multiple data types), it this case we should keep old properties for backward compatibility.

Currently DataTransfer does not contain a lot information in paste event, it works more with drop then paste. Also we are going to work on CKEditor 5 soon so it might be a solution to focus on drop event now and support rich paste event in CKE5.

Last edited 7 years ago by Piotr Jasiun (previous) (diff)

comment:5 Changed 7 years ago by Piotr Jasiun

After the #11526 research I think that drop handling should work this way:

  • when user drag contains inside editor mark it with some ID (we can find dragged content using selection on drag) and put this ID into dataTransfer,
  • when drop event occurred check if it is you ID;
    • if not: read html or text (if html is not available) and put in drop position,
    • if it is our ID: read html for our ID, remove dragger content and put html in the drop position.

In both situation we need to cancel a drop event. There are two thing we will lose with such solution. When user drag content from the outside of the editor:

  • on IE, where we have no html in dataTransfer object, content will be dropped as plain text,
  • we will lose native drag behavior so in every case drag content will not be removed.

comment:6 Changed 7 years ago by Piotrek Koszuliński

Based on PJ's research made in #11526 we discussed today the possible design of this feature. We agreed on most of points, although I feel that it's so complex and tricky that we need to have a "running spec" in which we'll be keeping track of all decisions.

Arguments and decisions

  1. Because of time limits and advanced age of 4.x branch, we cannot redesign entire input processing. Such steps can be done in future major release. We pointed out that it's unlikely that it will happen in CKEditor 5.x, because solution we're going to introduce in 4.4 will be satisfactory.
  2. IE 11 is as bad as IE 8 in terms of offered APIs and their completeness. Therefore, situation won't change significantly in couple of years.
  3. Current behaviour of editor#paste event must be preserved in as many details as possible.
  4. There should be a single entry point.
  5. If it's possible editor#paste should not be fired by editor#drop, what means that there would have to be a new event (editor#inputData).
  6. IEs do not allow to get HTML from drop and paste events. We still need pastebin for IEs, because we have to support pasting HTML from multiple sources. In case of drag&drop, we need to choose between preventing default drop (what would allow to handle it in a custom way) and being able to drag&drop HTML from outside editor into editor. We'll be able to handle drag&drop within editor (and even all editors on a page - although there's problem with things like filtering widgets in case of differently configured instances) by overriding carried data (only text format is available) by a reference to stored dragged HTML. Therefore, I think that we may accept the limitation that HTML is plain textified when drag&dropped from outside of editor.
  7. Drop and paste data cannot be read asynchronously (after timeout). Data from pastebin is read after short timeout after native paste... We've got a conflict of interests here.
  8. The additional data items that editor#paste and editor#drop events will carry should be facades to native DataTransfer objects. Theoretically we could cache data retrieved from native objects, but: a) it doesn't seem right, b) it's asynchronous what complicates the thing, c) we'll completely block possibility to use some File APIs (e.g. the ability to upload a blob rather than base64 encoded string to a server and couple of others like progress listener), d) it may be unnecessarily heavy. Therefore, to avoid such situation our data facade has to keep reference to native object.
  9. Because of two previous points we cannot use pastebin and offer items data at the same time. On problematic browsers like IE we can try to discover if paste may contain HTML (if getData('Text') returns something) and in this case use pastebin; if not, then we can skip pastebin and offer files data. Since, as explained in one of previous points, we may accept that only text can be dropped from outside of editor, that discovery has to be done for pasting only.
  10. If we wouldn't use pastebin at all on Blink/Webkit/Gecko (what's tempting because of bugs it causes and its complexity), we'll need to solve two problems:
    1. Webkit&Blink pastes/drops awful HTML overloaded with inline styles. The only possible way to handle this is by stripping all styles. This could be configurable, because in basic/standard presets ACF will strip all of them too, but by default such filter would have to enabled. There's also a light in the tunnel - perhaps we could override clipboard's data on copy/cut from editor with our, specially marked HTML string and then we wouldn't have to do a styles attribute filtering when copy/cut&paste was done within editor.
    2. There's no HTML version of dropped and pasted plain text. Currently we unify "HTMLified text" versions pasted by browsers into pastebin. Every browser generate different HTML, so we've got a hardcoded filters for discovery and transformations. We'll be able to get rid of most of it (leave only parts for IE, which surprisingly produces best HTML) and write simple "HTMLification processor" turning plain text into HTML.
  11. However, we may still use pastebin if no files are pasted (similar discovery as in IE case), so problems explained above would not exist. Unfortunately, they will still exist for dropping plain text and HTML, so we have to solve them anyway.
  12. Editor should add class to editable on dragover, so developers can add special styles to mark such editable.

Design

Data flow

  1. Step one - data capturing
    • The goal is to pass files list with data transfer facedes, HTML data string, and type to editor#paste and editor#drop.
    • There's a decision made whether we need to use pastebin or there are some files so we can't.
    • We retrieve data stored on dragstart (IE case).
    • We remove dragged content from editor.
    • If there was a plain text pasted/dropped we HTMLify it.
    • All data - HTML data string, type ('auto' by default, 'text' if it was plain text) and files list are passed to editor#drop or editor#paste.
  2. Step two - data processing
    • Includes: type discovery (text, html), plain text htmlification/normalization, html textification (forcePasteAsPlainText), MSWord documents normalization, etc.
    • We need this step during editor#paste and editor#drop, so we'll not need to change current editor#paste behaviour.
    • Methods used currently by editor#paste listeners have to be made public unless this part of editor#drop is implemented in the clipboard plugin.
  3. Step three - files processing
    • Can be done in editor#inputData, because it's a single entry point. Although, since the rest of processing happen on editor#drop and editor#paste we may consider processing files on them too.
    • During this step listeners check files list and if they can handle any of pasted/dropped files, they add HTML generated for that content to the HTML data string. For example "image handler" will add <img data-cke-file-id="rand" alt="" src="CKEDITOR.tools.transparentImageData"> and will replace src attribute with data URI retrieved (asynchronously!) from file, based on data-cke-file-id if it survived ACF and other filters. There has to be a way to hook file upload to that too. File upload might start at this point, so it will have access to data transfer facades.
    • It may be convenient to be able to mark that file was already "handled", so other listeners will not duplicate it.
    • It's unclear yet how image handler and uploader could communicate with image2, because replacing its source is not enough - data has to be updated. However, problem would not exist if that handled would be added by image2 itself.
  4. Step four - single entry point
    • editor#inputData is fired synchronously after execution of editor#drop and editor#paste with all their data (files lists, HTML data, type and custom properties).
    • It's fired in its 1000-prior listener, so editor#paste or editor#drop can be fired by other code and the chain will be finished.
  5. Step five - data insertion
    • If HTML data string is not empty, insertion is performed synchronously after editor#inputData (in its 1000-prior listener, so editor#inputData can be fired by other code and the chain will be finished).
    • editor#afterInputData is fired after insertion. Additionally editor#afterPaste is fired. Currently editor#afterPaste is fired asynchronously "so all other listeners for 'paste' will be fired first". It doesn't make sense, but on the other hand changing this would be hard, because synchronously after editor#paste we e.g. fire afterCommandExec.
    • editor#afterPaste and editor#beforePaste should be marked as deprecated. First is replaced by editor#afterInputData, second is only a problem for us and most likely is useless.
  6. Step six - files upload
    • Since at this point
    • After files data have been collected and HTML was inserted, file uploading begins. It may also begin earlier, when files are available, but it seems to be more correct to start uploading after insertion. However, in this case we need to find a way to cache files that might be uploaded, so their content (blob and dara URI) are still available here.
    • Elements referencing to those files are still marked with data-cke-file-id attribute.
    • Uploading file may take more time, so at every step we need to verify that these elements still exist (simply, by using document.find() instead of caching them).
    • Elements may be duplicated before uploading completes, so data-cke-file-id has to be preserved and situation when more than one element with this attribute exist has to be considered.
    • File id should be correlated with editor instance - when content was pasted into another editor, attribute must be removed and new upload thread has to be launched. The reason is that editors may have different configuration regarding files handling and uploading, so we cannot assume that it should be the same URI.

UI

  1. We may consider styling editable when class marking editable on dragover is present.
  2. We need a convenient place to communicate with user about uploading process. We thought about "toasts" - small notification boxes displayed in the bottom right corner of editable. Toasts system may use floatpanels to render and should have API to add notification and close it. Notification's content needs to be an HTML. We may implement simple version of toasts at the beginning (e.g. skip such tricky cases like keeping them in the viewport, when editable isn't fully visible and more options regarding positions). The behaviour (on hover, close, etc.) may be precised later and is not crucial now. In the future we could also use special toasts instead of alerts (there is a ticket about stopping using alerts).
  3. We are not considering displaying inline information about uploading progress because of undomanager, dirty flag and other complications.

Code separation

It's not yet clear how do we want to distribute code between plugins.

  • There may be developers which don't want even basic drop support (I don't think so, but there are e.g. touch devices where I'm unsure about drag&drop support). I would keep basic drop support in clipboard plugin because they'll share a lot of code anyway.
  • Where do we want to keep file handlers? We'll have at least one - for image, but we may have also for other files (which generates links). I think that they should be kept in image, image2 and link plugins. See also one of points in step three of data flow regarding communication with image2 widget.
  • Files uploading is a separate plugin.
  • Toasts are separate plugin.
  • What's left?
Last edited 7 years ago by Piotrek Koszuliński (previous) (diff)

comment:7 Changed 7 years ago by Piotr Jasiun

About clipboard facade and timeout. In fact it is not possible to read from clipboardData, dataTransfer or items (new API, Chrome). But we can easily save, for future usage, file references as files array or item.getAsFile() and read files contents whenever we need. I tested it and works fine in all cases (paste and drop in Chrome, FF and IE).

BTW I love when browsers use common API:

files = evt.dataTransfer ? evt.dataTransfer.files : // Drop
	evt.clipboardData && evt.clipboardData.items ? [ evt.clipboardData.items[0].getAsFile() ] : // Paste Chrome
	evt.clipboardData ? evt.clipboardData.files : // Paste Firefox
	window.clipboardData ? window.clipboardData.files : // Paste IE
	undefined;
Last edited 7 years ago by Piotr Jasiun (previous) (diff)

Changed 7 years ago by Piotr Jasiun

Attachment: keep-files.html added

comment:8 Changed 7 years ago by Piotr Jasiun

There a problem with drag position on IE, because:

  • on drag selection sometimes is in correct place but sometimes in totally wrong position or not exists at all,
  • range.moveToPoint method throws Unspecified error when you are not directly on contents (eg. cursor is next to the header); if I call it on mouseover I get more errors than positions and the last good position is usually far from expectation,
  • it is not possible to change native drop content so I can not change it to some hidden span and change after native drop.

My idea to solve this problem is to use elementFromPoint method which works pretty well even if cursor in next to the element. So if moveToPoint throws error I can get nearlest element using elementFromPoint and set the cousor at the begging or end of the element (based on cursor position).

Also we will need to fix dragged selection before we remove it, because if user select part of the table and drag and drop it, script will remove part of cells what causes crash on IE 11.

comment:9 Changed 7 years ago by Piotr Jasiun

Summary: The editor#drop event - handling dropping content in editorHandling dropping content in editor

comment:10 Changed 7 years ago by Piotr Jasiun

I implemented basic D&D and check how it works in browsers. There are numerous errors with getting drop position in browsers:

  • FF: wrong drop position when I drop at the end of the paragraph.
  • FF: "TypeError: startNode is null" when I drop at the end of the paragraph content from the same line.
  • IE8 & IE11: "unspecified error" (e.g. when I drop content next to the "Broadcasting and quotes" header.).
  • IE8 & IE11: "index size error" (specific case not yet investigated).
  • IE8: drag content does not disappears sometimes.
  • IE8: random crash.
  • IE8: D&D in the same paragraph does not work.
  • Chrome: everything is fine.

As long as we can fix most of this issue with simple workarounds, "unspecified error" on IE is the most popular error and the one we cannot easily avoid (for the reasons mentioned in the previous post).

So my proposal is to use custom code, based on document.elementFromPoint() and range.getClientRects() (which works surprisingly good in IE). I created prove of concept and it works on IE11-IE9 far better then native code. It is based on the standard range so it does not work on IE8 yet. I tested range.getClientRects() on IE8 and it works, so custom solution should work on IE8 as well.

To show you how both solution works there are them:

Of course the final version would be a mixed so it will use native methods when they works and custom when the do not. This is just a prove of concept. What do you think about it?

Last edited 6 years ago by Piotr Jasiun (previous) (diff)

comment:11 Changed 7 years ago by Piotrek Koszuliński

First of all we must have a really clean implementation for retrieving range from native sources on drop and retrieving selected HTML on dragstart. Otherwise we're not able to evaluate these parts. The current proposal in branch:t/11460-native contains already too much integration code which may cause issues and it makes testing of these separate parts harder.

For example I see that at some point you set selection after drop, but before retrieving the range at drop position. Such operation may have unpredictable influence on DOM and browsers' internal states. Then you changed the order of actions, but the situation is still far from being clear. You retrieve range, then play with selection and then try to use this range - that can be unpredictable.

Therefore, what we need is:

  1. Retrieving HTML on dragstart. The sample could log retrieved HTML on console.
  2. Remembering dragged selection. Since the standard bookmarks are more reliable, I would check if we couldn't use them instead of bookmarks2. Of course inserting spans may influence locating drop, but we'll check this in a next step. First we have to find out if we can make bookmarks on drag start and restore them on drop. Please make two versions of this test case - one using createBookmarks() and one using createBookmarks2(). After restoring ranges from these bookmarks use deleteContents() to see if all this worked.
  3. Retrieving drop position. Again - just retrieving, without previous steps. Insert some visible pointer (e.g. a letter) into that range. Check both methods - using the getRangeAtDropPosition borrowed from widget plugin and simple editor.getSelection().

The easiest way will be to create separate test files for all this cases. You don't need to push branches, just attach here sample HTML files.

PS. Remember that if you have found a bug in an existing method which you need to use, we have to fix that method first. You cannot proceed with a broken part of your implementation because we won't be able to evaluate later the entire component. Therefore, report tickets for such bugs, create test cases and notify me or Olek. Such ticket will be considered a blocker so we'll try to find a solution ASAP.

Changed 7 years ago by Piotr Jasiun

Keep drag selection with bookmark.

Changed 7 years ago by Piotr Jasiun

Keep drag selection with bookmark2.

Changed 7 years ago by Piotr Jasiun

Keep drag selection with ranges.

Changed 7 years ago by Piotr Jasiun

Attachment: readHtmlOnDragstart.html added

Read HTML on 'dragstart' and write it to the console.

Changed 7 years ago by Piotr Jasiun

Get drop position using native selection.

Changed 7 years ago by Piotr Jasiun

Get drop position using function borrowed from widgets.

Changed 7 years ago by Piotr Jasiun

Get drop position using custom script based on elementFromPoint and getClientRects

Changed 7 years ago by Piotr Jasiun

comment:12 Changed 7 years ago by Piotr Jasiun

Description: modified (diff)

I created set of samples for problems mentioned above.

  1. Retrieving HTML on dragstart.
    • readHtmlOnDragstart.html​. Works pretty well. Of course there are some edge cases (like fake selection) but these could be fixed later. I don't see any blocking issues here.
  2. Remembering dragged selection.

Both bookmark2 and range needs to be changed to the bookmark after drop, because we need to create a bookmark for a drop position which can destroy bookmark2 or range.

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