浏览代码

Merge branch 'master' into t/953

Piotrek Koszuliński 8 年之前
父节点
当前提交
31314bda13

+ 44 - 35
packages/ckeditor5-engine/src/dev-utils/enableenginedebug.js

@@ -62,7 +62,7 @@ const LOG_SEPARATOR = '-------';
 let enabled = false;
 
 // Logging function used to log debug messages.
-let log = console.log;
+let logger = console;
 
 /**
  * Enhances model classes with logging methods. Returns a plugin that should be loaded in the editor to
@@ -100,13 +100,12 @@ let log = console.log;
  * All those methods take one parameter, which is a version of {@link module:engine/model/document~Document model document}
  * for which model or view document state should be logged.
  *
- * @param {Function} [logger] Function used to log messages. By default messages are logged to console.
+ * @param {Object} [_logger] Object with functions used to log messages and errors. By default messages are logged to console.
+ * If specified, it is expected to have `log()` and `error()` methods.
  * @returns {module:engine/dev-utils/enableenginedebug~DebugPlugin} Plugin to be loaded in the editor.
  */
-export default function enableEngineDebug( logger ) {
-	if ( logger ) {
-		log = logger;
-	}
+export default function enableEngineDebug( _logger = console ) {
+	logger = _logger;
 
 	if ( !enabled ) {
 		enabled = true;
@@ -125,7 +124,7 @@ function enableLoggingTools() {
 	};
 
 	ModelPosition.prototype.log = function() {
-		log( 'ModelPosition: ' + this );
+		logger.log( 'ModelPosition: ' + this );
 	};
 
 	ModelRange.prototype.toString = function() {
@@ -133,7 +132,7 @@ function enableLoggingTools() {
 	};
 
 	ModelRange.prototype.log = function() {
-		log( 'ModelRange: ' + this );
+		logger.log( 'ModelRange: ' + this );
 	};
 
 	ModelText.prototype.toString = function() {
@@ -141,11 +140,11 @@ function enableLoggingTools() {
 	};
 
 	ModelText.prototype.logExtended = function() {
-		log( `ModelText: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
+		logger.log( `ModelText: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
 	};
 
 	ModelText.prototype.log = function() {
-		log( 'ModelText: ' + this );
+		logger.log( 'ModelText: ' + this );
 	};
 
 	ModelTextProxy.prototype.toString = function() {
@@ -153,11 +152,11 @@ function enableLoggingTools() {
 	};
 
 	ModelTextProxy.prototype.logExtended = function() {
-		log( `ModelTextProxy: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
+		logger.log( `ModelTextProxy: ${ this }, attrs: ${ mapString( this.getAttributes() ) }` );
 	};
 
 	ModelTextProxy.prototype.log = function() {
-		log( 'ModelTextProxy: ' + this );
+		logger.log( 'ModelTextProxy: ' + this );
 	};
 
 	ModelElement.prototype.toString = function() {
@@ -165,18 +164,18 @@ function enableLoggingTools() {
 	};
 
 	ModelElement.prototype.log = function() {
-		log( 'ModelElement: ' + this );
+		logger.log( 'ModelElement: ' + this );
 	};
 
 	ModelElement.prototype.logExtended = function() {
-		log( `ModelElement: ${ this }, ${ this.childCount } children, attrs: ${ mapString( this.getAttributes() ) }` );
+		logger.log( `ModelElement: ${ this }, ${ this.childCount } children, attrs: ${ mapString( this.getAttributes() ) }` );
 	};
 
 	ModelElement.prototype.logAll = function() {
-		log( '--------------------' );
+		logger.log( '--------------------' );
 
 		this.logExtended();
-		log( 'List of children:' );
+		logger.log( 'List of children:' );
 
 		for ( const child of this.getChildren() ) {
 			child.log();
@@ -216,7 +215,7 @@ function enableLoggingTools() {
 	};
 
 	ModelElement.prototype.logTree = function() {
-		log( this.printTree() );
+		logger.log( this.printTree() );
 	};
 
 	ModelRootElement.prototype.toString = function() {
@@ -224,7 +223,7 @@ function enableLoggingTools() {
 	};
 
 	ModelRootElement.prototype.log = function() {
-		log( 'ModelRootElement: ' + this );
+		logger.log( 'ModelRootElement: ' + this );
 	};
 
 	ModelDocumentFragment.prototype.toString = function() {
@@ -232,7 +231,7 @@ function enableLoggingTools() {
 	};
 
 	ModelDocumentFragment.prototype.log = function() {
-		log( 'ModelDocumentFragment: ' + this );
+		logger.log( 'ModelDocumentFragment: ' + this );
 	};
 
 	ModelDocumentFragment.prototype.printTree = function() {
@@ -262,11 +261,11 @@ function enableLoggingTools() {
 	};
 
 	ModelDocumentFragment.prototype.logTree = function() {
-		log( this.printTree() );
+		logger.log( this.printTree() );
 	};
 
 	Operation.prototype.log = function() {
-		log( this.toString() );
+		logger.log( this.toString() );
 	};
 
 	AttributeOperation.prototype.toString = function() {
@@ -306,11 +305,11 @@ function enableLoggingTools() {
 	};
 
 	Delta.prototype.log = function() {
-		log( this.toString() );
+		logger.log( this.toString() );
 	};
 
 	Delta.prototype.logAll = function() {
-		log( '--------------------' );
+		logger.log( '--------------------' );
 
 		this.log();
 
@@ -336,7 +335,17 @@ function enableLoggingTools() {
 	const _deltaTransformTransform = deltaTransform.transform;
 
 	deltaTransform.transform = function( a, b, isAMoreImportantThanB ) {
-		const results = _deltaTransformTransform( a, b, isAMoreImportantThanB );
+		let results;
+
+		try {
+			results = _deltaTransformTransform( a, b, isAMoreImportantThanB );
+		} catch ( e ) {
+			logger.error( 'Error during delta transformation!' );
+			logger.error( a.toString() + ( isAMoreImportantThanB ? ' (important)' : '' ) );
+			logger.error( b.toString() + ( isAMoreImportantThanB ? '' : ' (important)' ) );
+
+			throw e;
+		}
 
 		for ( let i = 0; i < results.length; i++ ) {
 			results[ i ]._saveHistory( {
@@ -424,11 +433,11 @@ function enableLoggingTools() {
 	};
 
 	ViewText.prototype.logExtended = function() {
-		log( 'ViewText: ' + this );
+		logger.log( 'ViewText: ' + this );
 	};
 
 	ViewText.prototype.log = function() {
-		log( 'ViewText: ' + this );
+		logger.log( 'ViewText: ' + this );
 	};
 
 	ViewTextProxy.prototype.toString = function() {
@@ -436,11 +445,11 @@ function enableLoggingTools() {
 	};
 
 	ViewTextProxy.prototype.logExtended = function() {
-		log( 'ViewTextProxy: ' + this );
+		logger.log( 'ViewTextProxy: ' + this );
 	};
 
 	ViewTextProxy.prototype.log = function() {
-		log( 'ViewTextProxy: ' + this );
+		logger.log( 'ViewTextProxy: ' + this );
 	};
 
 	ViewElement.prototype.printTree = function( level = 0 ) {
@@ -466,7 +475,7 @@ function enableLoggingTools() {
 	};
 
 	ViewElement.prototype.logTree = function() {
-		log( this.printTree() );
+		logger.log( this.printTree() );
 	};
 
 	ViewDocumentFragment.prototype.printTree = function() {
@@ -486,7 +495,7 @@ function enableLoggingTools() {
 	};
 
 	ViewDocumentFragment.prototype.logTree = function() {
-		log( this.printTree() );
+		logger.log( this.printTree() );
 	};
 }
 
@@ -511,7 +520,7 @@ function enableReplayerTools() {
 			return '';
 		}
 
-		const appliedDeltas = this._appliedDeltas.concat( this._lastDelta.toJSON() );
+		const appliedDeltas = this._appliedDeltas.concat( this._lastDelta );
 
 		return appliedDeltas.map( JSON.stringify ).join( LOG_SEPARATOR );
 	};
@@ -525,7 +534,7 @@ function enableDocumentTools() {
 	const _modelDocumentApplyOperation = ModelDocument.prototype.applyOperation;
 
 	ModelDocument.prototype.applyOperation = function( operation ) {
-		log( 'Applying ' + operation );
+		logger.log( 'Applying ' + operation );
 
 		if ( !this._operationLogs ) {
 			this._operationLogs = [];
@@ -564,12 +573,12 @@ function enableDocumentTools() {
 	};
 
 	function logDocument( document, version ) {
-		log( '--------------------' );
+		logger.log( '--------------------' );
 
 		if ( document[ treeDump ][ version ] ) {
-			log( document[ treeDump ][ version ] );
+			logger.log( document[ treeDump ][ version ] );
 		} else {
-			log( 'Tree log unavailable for given version: ' + version );
+			logger.log( 'Tree log unavailable for given version: ' + version );
 		}
 	}
 }

+ 11 - 0
packages/ckeditor5-engine/src/model/delta/attributedelta.js

@@ -90,6 +90,17 @@ export default class AttributeDelta extends Delta {
 	/**
 	 * @inheritDoc
 	 */
+	toJSON() {
+		const json = super.toJSON();
+
+		delete json._range;
+
+		return json;
+	}
+
+	/**
+	 * @inheritDoc
+	 */
 	static get className() {
 		return 'engine.model.delta.AttributeDelta';
 	}

+ 9 - 0
packages/ckeditor5-engine/src/view/domconverter.js

@@ -18,6 +18,7 @@ import ViewDocumentFragment from './documentfragment';
 import ViewTreeWalker from './treewalker';
 import { BR_FILLER, INLINE_FILLER_LENGTH, isBlockFiller, isInlineFiller, startsWithFiller, getDataWithoutFiller } from './filler';
 
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 import indexOf from '@ckeditor/ckeditor5-utils/src/dom/indexof';
 import getAncestors from '@ckeditor/ckeditor5-utils/src/dom/getancestors';
 import getCommonAncestor from '@ckeditor/ckeditor5-utils/src/dom/getcommonancestor';
@@ -735,7 +736,15 @@ export default class DomConverter {
 		const domEditable = this.getCorrespondingDomElement( viewEditable );
 
 		if ( domEditable && domEditable.ownerDocument.activeElement !== domEditable ) {
+			const { scrollX, scrollY } = global.window;
+			const { scrollLeft, scrollTop } = domEditable;
+
 			domEditable.focus();
+
+			// https://github.com/ckeditor/ckeditor5-engine/issues/951
+			domEditable.scrollLeft = scrollLeft;
+			domEditable.scrollTop = scrollTop;
+			global.window.scrollTo( scrollX, scrollY );
 		}
 	}
 

+ 23 - 4
packages/ckeditor5-engine/tests/dev-utils/enableenginedebug.js

@@ -61,7 +61,7 @@ describe( 'enableEngineDebug', () => {
 } );
 
 describe( 'debug tools', () => {
-	let DebugPlugin, log;
+	let DebugPlugin, log, error;
 
 	class TestEditor extends StandardEditor {
 		constructor( ...args ) {
@@ -74,7 +74,8 @@ describe( 'debug tools', () => {
 
 	before( () => {
 		log = sinon.spy();
-		DebugPlugin = enableEngineDebug( log );
+		error = sinon.spy();
+		DebugPlugin = enableEngineDebug( { log, error } );
 	} );
 
 	afterEach( () => {
@@ -765,7 +766,7 @@ describe( 'debug tools', () => {
 		} );
 	} );
 
-	describe( 'should provide means for saving delta history transformation', () => {
+	describe( 'should provide debug tools for delta transformation', () => {
 		let document, root, otherRoot;
 
 		beforeEach( () => {
@@ -891,7 +892,7 @@ describe( 'debug tools', () => {
 			} );
 		} );
 
-		it( 'recreate delta using history', () => {
+		it( 'recreate delta using Delta#history', () => {
 			const deltaA = new MoveDelta();
 			const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
 			deltaA.addOperation( opA );
@@ -930,6 +931,24 @@ describe( 'debug tools', () => {
 
 			expect( JSON.stringify( recreated ) ).to.equal( JSON.stringify( original ) );
 		} );
+
+		it( 'provide additional logging when transformation crashes', () => {
+			const deltaA = new MoveDelta();
+			const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
+			deltaA.addOperation( opA );
+
+			const deltaB = new InsertDelta();
+			const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 );
+			deltaB.addOperation( opB );
+
+			deltaTransform.defaultTransform = () => {
+				throw new Error();
+			};
+
+			expect( () => deltaTransform.transform( deltaA, deltaB, true ) ).to.throw( Error );
+			expect( error.calledWith( deltaA.toString() + ' (important)' ) ).to.be.true;
+			expect( error.calledWith( deltaB.toString() ) ).to.be.true;
+		} );
 	} );
 
 	function expectLog( expectedLogMsg ) {

+ 1 - 1
packages/ckeditor5-engine/tests/dev-utils/model.js

@@ -114,7 +114,7 @@ describe( 'model test utils', () => {
 			test( '[this is test] text' );
 		} );
 
-		it( 'should insert text with selection inside #2', () => {
+		it( 'should insert text with selection inside #3', () => {
 			test( 'this is [test text]' );
 		} );
 

+ 7 - 0
packages/ckeditor5-engine/tests/model/delta/attributedelta.js

@@ -11,6 +11,7 @@ import Position from '../../../src/model/position';
 import Element from '../../../src/model/element';
 import { default as AttributeDelta, RootAttributeDelta } from '../../../src/model/delta/attributedelta';
 import AttributeOperation from '../../../src/model/operation/attributeoperation';
+import { jsonParseStringify } from '../../../tests/model/_utils/utils';
 
 describe( 'Batch', () => {
 	let batch, doc, root;
@@ -505,6 +506,12 @@ describe( 'AttributeDelta', () => {
 	it( 'should provide proper className', () => {
 		expect( AttributeDelta.className ).to.equal( 'engine.model.delta.AttributeDelta' );
 	} );
+
+	it( 'should not have _range property when converted to JSON', () => {
+		const json = jsonParseStringify( delta );
+
+		expect( json ).not.to.have.property( '_range' );
+	} );
 } );
 
 describe( 'RootAttributeDelta', () => {

+ 27 - 0
packages/ckeditor5-engine/tests/view/domconverter/domconverter.js

@@ -10,6 +10,7 @@ import ViewEditable from '../../../src/view/editableelement';
 import ViewDocument from '../../../src/view/document';
 import { BR_FILLER, NBSP_FILLER } from '../../../src/view/filler';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 
 testUtils.createSinonSandbox();
 
@@ -66,6 +67,32 @@ describe( 'DomConverter', () => {
 
 			expect( focusSpy.calledOnce ).to.be.true;
 		} );
+
+		// https://github.com/ckeditor/ckeditor5-engine/issues/951
+		it( 'should actively prevent window scroll in WebKit', () => {
+			const scrollToSpy = testUtils.sinon.stub( global.window, 'scrollTo' );
+			const scrollLeftSpy = sinon.spy();
+			const scrollTopSpy = sinon.spy();
+
+			Object.defineProperties( domEditable, {
+				scrollLeft: {
+					get: () => 20,
+					set: scrollLeftSpy
+				},
+				scrollTop: {
+					get: () => 200,
+					set: scrollTopSpy
+				}
+			} );
+
+			global.window.scrollX = 10;
+			global.window.scrollY = 100;
+
+			converter.focus( viewEditable );
+			sinon.assert.calledWithExactly( scrollToSpy, 10, 100 );
+			sinon.assert.calledWithExactly( scrollLeftSpy, 20 );
+			sinon.assert.calledWithExactly( scrollTopSpy, 200 );
+		} );
 	} );
 
 	describe( 'DOM nodes type checking', () => {