Improve dataTransfer object

[Piotr Jasiun]

Part of #11437.

setData and getData for custom data types should work on every browser. dataTransfer object should also have isDrag property.

[Piotr Jasiun]

• reorganized event data and dataTransfer structure so now it looks like this:

  {
    dataValue:            string,
    type:                 'auto|text|html',
    [method]:               'drop|undefined',
    [dataTransfer]: {
        id:               string,
        sourceEditor:     editor,
        targetEditor:     editor,
        getData:          function,
        setData:          function,
        getTransferType:  function, // returns external|internal|cross editor
    }
  }
  

• improved setData and getData so now they support custom data types on every browser,
• dataTransfer may have no native object,
• used code from insertText to transform text to html instead of simple htmlEncode.

Now dataVaule and type are set, based on dataTransfer data, just before firing paste. It will be moved to the paste listener in #12173, but now paste event is not firing if dataValue is empty so we need to set it earlier.

Changes in ​t/12168.

[Piotrek Koszuliński]

• textToHtml
  • Name transformPlainTextToHtml (the same in the method's doc). BTW. Docs should be written in 3rd person - i.e. 'transforms' instead of 'transform'.
  • This method bases its behaviour on two parameters - selection's position and enter mode. I see now that it does not make much sense in editable class, because e.g. it's impossible to use it for selection placed in other place than where selection is currently placed. Therefore, please move it to tools and make the 2nd arguments options object (with properties: enterMode, inPre).
  • Where are tests for this method?
• I think that our dataTransfer interface should suppress native errors. The getData method should use try-catch and return null or empty string (depending on how it works natively) if error was thrown on IE. Specifically to avoid this: ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1351. On the other hand try-catch is already used here: ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1876 so it may be just a bug.
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1358 - don't set it on the object if you are going to change it few lines below. When reading this I wanted to comment that - "wait, traveler, you forgot to transform plain text to HTML". Only after few whiles I noticed what happens below. Use a local variable while still processing it.
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1369 - fix indentation.
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1821 - empty line missing.
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1736 - extract this method from the constructor.
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1736 - why private?
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1897 - here and in other places don't use variable t. Use type from the first moment, because you don't need t after normalising it.
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/plugins/clipboard/plugin.js#L1904 - method could return true/false depending on whether data was set. It could be a useful feedback.
• Split setData getData tests into multiple, properly titled, test cases.
• ​https://github.com/cksource/ckeditor-dev/blob/89063ac575cf110d9b0a6387f3f983ce7a9e33b2/tests/plugins/clipboard/datatransfer.js#L289 - avoid setting such HTML, because it's incorrect and will be auto paragraphed. This leads to tests being less clear and of course bugs.

[Piotr Jasiun]

Changes done and pushed.

transformPlainTextToHtml needs only enterMode as an option so I decided no to create option object for one option. In the future if we will have more options we can check the type of the parameter.

dataTransfer.setData will always work, even if native method will not, so it is pointless to return result.

[Piotr Jasiun]

Rebased on newest major.

[Piotrek Koszuliński]

1. You forgot to remove editable#textToHtml method.
2. I see that the code which decides which enter mode should be used while transforming text o HTML is repeated twice - in clipboard plugin and in editable. This means that we need editable#transformPlainTextToHtml which will do this and which will use tools.transformPlainTextToHtml.
3. You forgot the @since tags.
4. ​https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L382 - don't mention paste position, because you don't know context in which this method will be used - it'd a general purpose transform function.
5. ​https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L383 - "Transformed" indicates that this is the HTML that has been transformed, while, in real, it was generated.
6. ​https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L383 - start comment from the upper case.
7. ​https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L467 - unnecessary semicolon. Actually, there are 5 linter errors.
8. tools#getUniqueId does not have tests. Please write at least one that will verify e.g. type of returned value and its length.

If any of these points conflicts with code created in branches based we can extract it to a separate ticket and fix afterwards, because none is critical.

[Piotr Jasiun]

Changes done and pushed to ​t/12168.

[Piotr Jasiun]

Closed with hash ​git:cb28e9c.