Skip to content

Commit

Permalink
Review fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
@f1ames
f1ames committed Nov 8, 2017
1 parent 3375736 commit e61ac60
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 41 deletions.
29 changes: 13 additions & 16 deletions plugins/clipboard/plugin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -2176,6 +2172,7 @@
}
}
};
this._.fallbackDataTransfer = new CKEDITOR.plugins.clipboard.fallbackDataTransfer( this );

// Check if ID is already created.
this.id = this.getData( clipboardIdDataType );
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -2437,7 +2434,7 @@

var data = that.getData( type, !that._.fallbackDataTransfer.isRequired() );
if ( data ) {
that._.cache[ type ] = data;
that._.data[ type ] = data;
}
}

Expand Down Expand Up @@ -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;
} );

Expand Down Expand Up @@ -2589,26 +2586,26 @@
* @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.
*
* @private
* @property {Object} _nativeDataTransfer
*/
this._nativeDataTransfer = nativeDataTransfer;
this._nativeDataTransfer = dataTransfer.$;

/**
* A MIME type used for storing custom MIME types.
Expand Down Expand Up @@ -2642,7 +2639,7 @@
*
* @private
* @static
* @property {Array}
* @property {String[]}
*/
CKEDITOR.plugins.clipboard.fallbackDataTransfer._customTypes = [];

Expand Down
21 changes: 14 additions & 7 deletions tests/plugins/clipboard/datatransfer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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>' );

Expand Down Expand Up @@ -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' );
Expand All @@ -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' );
Expand All @@ -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>' );

Expand Down Expand Up @@ -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' );
},
Expand All @@ -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' );
}
} );
37 changes: 19 additions & 18 deletions tests/plugins/clipboard/fallbackdatatransfer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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 );
Expand All @@ -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 );
Expand All @@ -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 );

Expand All @@ -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 );

Expand All @@ -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 );

Expand All @@ -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 );

Expand All @@ -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 );

Expand All @@ -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 );

Expand All @@ -374,15 +374,15 @@ 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 );
assert.areSame( '', extracted.content );
},

'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 );
Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand Down

0 comments on commit e61ac60

Please sign in to comment.