From e61ac60b4d6b3f3123943af9f4d875331432c4c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= <k.krzton@cksource.com>
Date: Wed, 8 Nov 2017 16:09:50 +0100
Subject: [PATCH] Review fixes.

---
 plugins/clipboard/plugin.js                   | 29 +++++++--------
 tests/plugins/clipboard/datatransfer.js       | 21 +++++++----
 .../plugins/clipboard/fallbackdatatransfer.js | 37 ++++++++++---------
 3 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/plugins/clipboard/plugin.js b/plugins/clipboard/plugin.js
index ed03d52ebd4..ea1d52a9df8 100644
--- a/plugins/clipboard/plugin.js
+++ b/plugins/clipboard/plugin.js
@@ -2152,17 +2152,13 @@
 			this.$ = nativeDataTransfer;
 		}
 
-		var cache = {};
-
 		this._ = {
 			metaRegExp: /^<meta.*?>/i,
 			bodyRegExp: /<body(?:[\s\S]*?)>([\s\S]*)<\/body>/i,
 			fragmentRegExp: /<!--(?:Start|End)Fragment-->/g,
 
+			data: {},
 			files: [],
-			cache: cache,
-
-			fallbackDataTransfer: new CKEDITOR.plugins.clipboard.fallbackDataTransfer( cache, this.$ ),
 
 			normalizeType: function( type ) {
 				type = type.toLowerCase();
@@ -2176,6 +2172,7 @@
 				}
 			}
 		};
+		this._.fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( this );
 
 		// Check if ID is already created.
 		this.id = this.getData( clipboardIdDataType );
@@ -2311,7 +2308,7 @@
 					data = '';
 				}
 			} else {
-				data = this._.cache[ type ] || null;
+				data = this._.data[ type ] || null;
 				if ( isEmpty( data ) ) {
 					if ( this._.fallbackDataTransfer.isRequired() ) {
 						data = this._.fallbackDataTransfer.getData( type );
@@ -2364,7 +2361,7 @@
 		setData: function( type, value ) {
 			type = this._.normalizeType( type );
 
-			this._.cache[ type ] = value;
+			this._.data[ type ] = value;
 
 			// There is "Unexpected call to method or property access." error if you try
 			// to set data of unsupported type on IE.
@@ -2437,7 +2434,7 @@
 
 				var data = that.getData( type, !that._.fallbackDataTransfer.isRequired() );
 				if ( data ) {
-					that._.cache[ type ] = data;
+					that._.data[ type ] = data;
 				}
 			}
 
@@ -2523,7 +2520,7 @@
 				return false;
 			}
 
-			CKEDITOR.tools.array.forEach( CKEDITOR.tools.objectKeys( this._.cache ), function( type ) {
+			CKEDITOR.tools.array.forEach( CKEDITOR.tools.objectKeys( this._.data ), function( type ) {
 				typesToCheck[ type ] = 1;
 			} );
 
@@ -2589,18 +2586,18 @@
 	 * @since 4.8.0
 	 * @class CKEDITOR.plugins.clipboard.fallbackDataTransfer
 	 * @constructor
-	 * @param {Object} cache Cache object used on storing and retrieving data.
-	 * @param {Object} [nativeDataTransfer] A native data transfer object.
+	 * @param {CKEDITOR.plugins.clipboard.dataTransfer} dataTransfer DataTransfer
+	 * object which internal cache and
+	 * {@link CKEDITOR.plugins.clipboard.dataTransfer#$ data transfer} objects will be reused.
 	 */
-	CKEDITOR.plugins.clipboard.fallbackDataTransfer = function( cache, nativeDataTransfer ) {
-
+	CKEDITOR.plugins.clipboard.fallbackDataTransfer = function( dataTransfer ) {
 		/**
 		 * Cache object. Shared with {@link CKEDITOR.plugins.clipboard.dataTransfer} instance.
 		 *
 		 * @private
 		 * @property {Object} _cache
 		 */
-		this._cache = cache;
+		this._cache = dataTransfer._.data;
 
 		/**
 		 * A native dataTransfer object.
@@ -2608,7 +2605,7 @@
 		 * @private
 		 * @property {Object} _nativeDataTransfer
 		 */
-		this._nativeDataTransfer = nativeDataTransfer;
+		this._nativeDataTransfer = dataTransfer.$;
 
 		/**
 		 * A MIME type used for storing custom MIME types.
@@ -2642,7 +2639,7 @@
 	 *
 	 * @private
 	 * @static
-	 * @property {Array}
+	 * @property {String[]}
 	 */
 	CKEDITOR.plugins.clipboard.fallbackDataTransfer._customTypes = [];
 
diff --git a/tests/plugins/clipboard/datatransfer.js b/tests/plugins/clipboard/datatransfer.js
index 3d5d1bb6451..4a7fe10af0c 100644
--- a/tests/plugins/clipboard/datatransfer.js
+++ b/tests/plugins/clipboard/datatransfer.js
@@ -36,7 +36,9 @@ bender.test( {
 	'test id': function() {
 		var nativeData1 = bender.tools.mockNativeDataTransfer(),
 			nativeData2 = bender.tools.mockNativeDataTransfer(),
-			dataTransfer1a, dataTransfer1b, dataTransfer2;
+			dataTransfer1a,
+			dataTransfer1b,
+			dataTransfer2;
 
 		// Setting id was moved from dataTransfer constructor to functions which initializes dataTransfer object
 		// only on specific events so we need to simulate these behaviour here too (#962).
@@ -60,7 +62,8 @@ bender.test( {
 	'test internal drag drop': function() {
 		var bot = this.editorBots.editor1,
 			editor = this.editors.editor1,
-			nativeData, dataTransfer;
+			nativeData,
+			dataTransfer;
 
 		bot.setHtmlWithSelection( '<p>x[x<b>foo</b>x]x</p>' );
 
@@ -99,7 +102,8 @@ bender.test( {
 
 	'test drop text from external source': function() {
 		var editor = this.editors.editor1,
-			nativeData, dataTransfer;
+			nativeData,
+			dataTransfer;
 
 		nativeData = bender.tools.mockNativeDataTransfer();
 		nativeData.setData( 'Text', 'x<b>foo</b>x' );
@@ -118,7 +122,8 @@ bender.test( {
 	'test drop html from external source': function() {
 		var isCustomDataTypesSupported = CKEDITOR.plugins.clipboard.isCustomDataTypesSupported,
 			editor = this.editors.editor1,
-			nativeData, dataTransfer;
+			nativeData,
+			dataTransfer;
 
 		nativeData = bender.tools.mockNativeDataTransfer();
 		nativeData.setData( 'Text', 'bar' );
@@ -141,7 +146,8 @@ bender.test( {
 		var bot1 = this.editorBots.editor1,
 			editor1 = this.editors.editor1,
 			editor2 = this.editors.editor2,
-			nativeData, dataTransfer;
+			nativeData,
+			dataTransfer;
 
 		bot1.setHtmlWithSelection( '<p>x[x<b>foo</b>x]x</p>' );
 
@@ -1068,7 +1074,7 @@ bender.test( {
 	},
 
 	'test if cache is initialized on dataTransfer creation': function() {
-		var cache = new CKEDITOR.plugins.clipboard.dataTransfer()._.cache;
+		var cache = new CKEDITOR.plugins.clipboard.dataTransfer()._.data;
 
 		assert.isObject( cache, 'cache should be initialized' );
 	},
@@ -1077,6 +1083,7 @@ bender.test( {
 		var dt1 = new CKEDITOR.plugins.clipboard.dataTransfer(),
 			dt2 = new CKEDITOR.plugins.clipboard.dataTransfer();
 
-		assert.isTrue( dt1._.cache !== dt2._.cache, 'caches should not be equal' );
+		assert.isObject( dt1._.data, 'cache should be initialized' );
+		assert.isTrue( dt1._.data !== dt2._.data, 'caches should not be equal' );
 	}
 } );
diff --git a/tests/plugins/clipboard/fallbackdatatransfer.js b/tests/plugins/clipboard/fallbackdatatransfer.js
index c4d3aaf4a73..00b3cb78ab9 100644
--- a/tests/plugins/clipboard/fallbackdatatransfer.js
+++ b/tests/plugins/clipboard/fallbackdatatransfer.js
@@ -265,35 +265,35 @@ bender.test( {
 	},
 
 	'test _applyDataComment case1': function() {
-		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			expected = document.querySelector( '#case1' ).innerHTML;
 
 		this.assertApplyDataComment( '<h1>Header1</h1><p>Test1</p>', { test: 1 }, dataTransferFallback, expected );
 	},
 
 	'test _applyDataComment case2': function() {
-		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			expected = document.querySelector( '#case2' ).innerHTML;
 
 		this.assertApplyDataComment( '<h1>Header1</h1><p>Test1</p>', { test: 1, comment: '<!-- comment -->' }, dataTransferFallback, expected );
 	},
 
 	'test _applyDataComment case3': function() {
-		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			expected = CKEDITOR.tools.trim( document.querySelector( '#case3' ).innerHTML );
 
 		this.assertApplyDataComment( '<!-- Start Comment --><h1>Header1</h1><p>Test1</p>', { test: 1 }, dataTransferFallback, expected );
 	},
 
 	'test _applyDataComment case4': function() {
-		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			expected = CKEDITOR.tools.trim( document.querySelector( '#case4' ).innerHTML );
 
 		this.assertApplyDataComment( '<h1>Header1</h1><p>Test1</p><!-- End Comment -->', { test: 123 }, dataTransferFallback, expected );
 	},
 
 	'test _applyDataComment with empty content': function() {
-		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			expected = CKEDITOR.tools.trim( document.querySelector( '#empty-content' ).innerHTML );
 
 		this.assertApplyDataComment( undefined, { test: 1 }, dataTransferFallback, expected );
@@ -302,7 +302,7 @@ bender.test( {
 	},
 
 	'test _applyDataComment with empty data': function() {
-		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			expected = CKEDITOR.tools.trim( document.querySelector( '#empty-data' ).innerHTML );
 
 		this.assertApplyDataComment( '<p>foobar</p>', '', dataTransferFallback, expected );
@@ -311,7 +311,7 @@ bender.test( {
 	},
 
 	'test _applyDataComment with empty content and data': function() {
-		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var dataTransferFallback = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			expected = '';
 
 		this.assertApplyDataComment( undefined, null, dataTransferFallback, expected );
@@ -320,7 +320,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment case1': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			content = document.querySelector( '#case1' ).innerHTML,
 			extracted = fallbackDataTransfer._extractDataComment( content );
 
@@ -329,7 +329,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment case2': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			content = document.querySelector( '#case2' ).innerHTML,
 			extracted = fallbackDataTransfer._extractDataComment( content );
 
@@ -338,7 +338,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment case3': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			content = document.querySelector( '#case3' ).innerHTML,
 			extracted = fallbackDataTransfer._extractDataComment( content );
 
@@ -347,7 +347,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment case4': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			content = document.querySelector( '#case4' ).innerHTML,
 			extracted = fallbackDataTransfer._extractDataComment( content );
 
@@ -356,7 +356,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment with empty content': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			content = CKEDITOR.tools.trim( document.querySelector( '#empty-content' ).innerHTML ),
 			extracted = fallbackDataTransfer._extractDataComment( content );
 
@@ -365,7 +365,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment with empty data': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			content = CKEDITOR.tools.trim( document.querySelector( '#empty-data' ).innerHTML ),
 			extracted = fallbackDataTransfer._extractDataComment( content );
 
@@ -374,7 +374,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment with empty data and content': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null ),
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } ),
 			extracted = fallbackDataTransfer._extractDataComment( '' );
 
 		assert.isNull( extracted.data );
@@ -382,7 +382,7 @@ bender.test( {
 	},
 
 	'test _extractDataComment with falsy value': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null );
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { _: { data: {} } } );
 
 		assert.areSame( '', fallbackDataTransfer._extractDataComment( '' ).content );
 		assert.areSame( '', fallbackDataTransfer._extractDataComment( null ).content );
@@ -391,7 +391,8 @@ bender.test( {
 	},
 
 	'test if isRequired sets _isCustomMimeTypeSupported flag on the first run': function() {
-		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null, bender.tools.mockNativeDataTransfer() );
+		var fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer(
+			{ $: bender.tools.mockNativeDataTransfer(), _: { data: {} } } );
 
 		CKEDITOR.plugins.clipboard.fallbackDataTransfer._isCustomMimeTypeSupported = null;
 
@@ -405,7 +406,7 @@ bender.test( {
 
 	'test if isRequired clears test MIME type': function() {
 		var nativeData = bender.tools.mockNativeDataTransfer(),
-			fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null, nativeData );
+			fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { $: nativeData, _: { data: {} } } );
 
 		CKEDITOR.plugins.clipboard.fallbackDataTransfer._isCustomMimeTypeSupported = null;
 
@@ -417,7 +418,7 @@ bender.test( {
 
 	'test if isRequired does not remove other MIME types': function() {
 		var nativeData = bender.tools.mockNativeDataTransfer(),
-			fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( null, nativeData );
+			fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( { $: nativeData, _: { data: {} } } );
 
 		CKEDITOR.plugins.clipboard.fallbackDataTransfer._isCustomMimeTypeSupported = null;