Merge pull request #1760 from ckeditor/t/ckeditor5/383

Internal: Removed usage of the CKEditor 5 logger. Part of ckeditor/ckeditor5#383.

src/conversion/downcasthelpers.js

@@ -3,6 +3,12 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
+/**
+ * Contains downcast (model-to-view) converters for {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher}.
+ *
+ * @module engine/conversion/downcasthelpers
+ */
+
 import ModelRange from '../model/range';
 import ModelSelection from '../model/selection';
 import ModelElement from '../model/element';
@@ -11,14 +17,8 @@ import ViewAttributeElement from '../view/attributeelement';
 import DocumentSelection from '../model/documentselection';
 import ConversionHelpers from './conversionhelpers';
 
-import log from '@ckeditor/ckeditor5-utils/src/log';
 import { cloneDeep } from 'lodash-es';
-
-/**
- * Contains downcast (model-to-view) converters for {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher}.
- *
- * @module engine/conversion/downcasthelpers
- */
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
  * Downcast conversion helper functions.
@@ -823,10 +823,11 @@ function changeAttribute( attributeCreator ) {
 			 *
 			 * @error conversion-attribute-to-attribute-on-text
 			 */
-			log.warn( 'conversion-attribute-to-attribute-on-text: ' +
-				'Trying to convert text node\'s attribute with attribute-to-attribute converter.' );
-
-			return;
+			throw new CKEditorError(
+				'conversion-attribute-to-attribute-on-text: ' +
+				'Trying to convert text node\'s attribute with attribute-to-attribute converter.',
+				[ data, conversionApi ]
+			);
 		}
 
 		// First remove the old attribute if there was one.

src/model/documentselection.js

@@ -17,7 +17,6 @@ import TextProxy from './textproxy';
 import toMap from '@ckeditor/ckeditor5-utils/src/tomap';
 import Collection from '@ckeditor/ckeditor5-utils/src/collection';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 import uid from '@ckeditor/ckeditor5-utils/src/uid';
 
 const storePrefix = 'selection:';
@@ -804,12 +803,7 @@ class LiveSelection extends Selection {
 		this._checkRange( range );
 
 		if ( range.root == this._document.graveyard ) {
-			/**
-			 * Trying to add a Range that is in the graveyard root. Range rejected.
-			 *
-			 * @warning model-selection-range-in-graveyard
-			 */
-			log.warn( 'model-selection-range-in-graveyard: Trying to add a Range that is in the graveyard root. Range rejected.' );
+			// @if CK_DEBUG // console.warn( 'Trying to add a Range that is in the graveyard root. Range rejected.' );
 
 			return;
 		}

src/model/operation/transform.js

@@ -16,7 +16,6 @@ import Range from '../range';
 import Position from '../position';
 
 import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 
 const transformations = new Map();
 
@@ -102,14 +101,14 @@ export function transform( a, b, context = {} ) {
 
 		return transformationFunction( a, b, context );
 	} catch ( e ) {
-		log.error( 'Error during operation transformation!', e.message );
-		log.error( 'Transformed operation', a );
-		log.error( 'Operation transformed by', b );
-		log.error( 'context.aIsStrong', context.aIsStrong );
-		log.error( 'context.aWasUndone', context.aWasUndone );
-		log.error( 'context.bWasUndone', context.bWasUndone );
-		log.error( 'context.abRelation', context.abRelation );
-		log.error( 'context.baRelation', context.baRelation );
+		// @if CK_DEBUG // console.error( 'Error during operation transformation!', e.message );
+		// @if CK_DEBUG // console.error( 'Transformed operation', a );
+		// @if CK_DEBUG // console.error( 'Operation transformed by', b );
+		// @if CK_DEBUG // console.error( 'context.aIsStrong', context.aIsStrong );
+		// @if CK_DEBUG // console.error( 'context.aWasUndone', context.aWasUndone );
+		// @if CK_DEBUG // console.error( 'context.bWasUndone', context.bWasUndone );
+		// @if CK_DEBUG // console.error( 'context.abRelation', context.abRelation );
+		// @if CK_DEBUG // console.error( 'context.baRelation', context.baRelation );
 
 		throw e;
 	}

src/model/utils/insertcontent.js

@@ -11,9 +11,9 @@ import Position from '../position';
 import LivePosition from '../liveposition';
 import Element from '../element';
 import Range from '../range';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 import DocumentSelection from '../documentselection';
 import Selection from '../selection';
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
  * Inserts content into the editor (specified selection) as one would expect the paste
@@ -85,13 +85,8 @@ export default function insertContent( model, content, selectable, placeOrOffset
 		} else {
 			// We are not testing else because it's a safe check for unpredictable edge cases:
 			// an insertion without proper range to select.
-
-			/**
-			 * Cannot determine a proper selection range after insertion.
-			 *
-			 * @warning insertcontent-no-range
-			 */
-			log.warn( 'insertcontent-no-range: Cannot determine a proper selection range after insertion.' );
+			//
+			// @if CK_DEBUG // console.warn( 'Cannot determine a proper selection range after insertion.' );
 		}
 
 		const affectedRange = insertion.getAffectedRange() || model.createRange( insertionPosition );
@@ -322,12 +317,19 @@ class Insertion {
 		if ( !this.schema.checkChild( this.position, node ) ) {
 			// Algorithm's correctness check. We should never end up here but it's good to know that we did.
 			// Note that it would often be a silent issue if we insert node in a place where it's not allowed.
-			log.error(
-				'insertcontent-wrong-position: The node cannot be inserted on the given position.',
+
+			/**
+			 * Given node cannot be inserted on the given position.
+			 *
+			 * @error insertcontent-wrong-position
+			 * @param {module:engine/model/node~Node} node Node to insert.
+			 * @param {module:engine/model/position~Position} position Position to insert the node at.
+			 */
+			throw new CKEditorError(
+				'insertcontent-wrong-position: Given node cannot be inserted on the given position.',
+				this,
 				{ node, position: this.position }
 			);
-
-			return;
 		}
 
 		const livePos = LivePosition.fromPosition( this.position, 'toNext' );
@@ -442,7 +444,13 @@ class Insertion {
 				// Algorithm's correctness check. We should never end up here but it's good to know that we did.
 				// At this point the insertion position should be after the node we'll merge. If it isn't,
 				// it should need to be secured as in the left merge case.
-				log.error( 'insertcontent-wrong-position-on-merge: The insertion position should equal the merge position' );
+				/**
+				 * An internal error occured during merging insertion content with siblings.
+				 * The insertion position should equal to the merge position.
+				 *
+				 * @error insertcontent-invalid-insertion-position
+				 */
+				throw new CKEditorError( 'insertcontent-invalid-insertion-position', this );
 			}
 
 			// Move the position to the previous node, so it isn't moved to the graveyard on merge.

src/view/observer/selectionobserver.js

@@ -11,7 +11,6 @@
 
 import Observer from './observer';
 import MutationObserver from './mutationobserver';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 import { debounce } from 'lodash-es';
 
 /**
@@ -150,15 +149,12 @@ export default class SelectionObserver extends Observer {
 		// This counter is reset each second. 60 selection changes in 1 second is enough high number
 		// to be very difficult (impossible) to achieve using just keyboard keys (during normal editor use).
 		if ( ++this._loopbackCounter > 60 ) {
-			/**
-			 * Selection change observer detected an infinite rendering loop.
-			 * Most probably you try to put the selection in the position which is not allowed
-			 * by the browser and browser fixes it automatically what causes `selectionchange` event on
-			 * which a loopback through a model tries to re-render the wrong selection and again.
-			 *
-			 * @error selectionchange-infinite-loop
-			 */
-			log.warn( 'selectionchange-infinite-loop: Selection change observer detected an infinite rendering loop.' );
+			// Selection change observer detected an infinite rendering loop.
+			// Most probably you try to put the selection in the position which is not allowed
+			// by the browser and browser fixes it automatically what causes `selectionchange` event on
+			// which a loopback through a model tries to re-render the wrong selection and again.
+			//
+			// @if CK_DEBUG // console.warn( 'Selection change observer detected an infinite rendering loop.' );
 
 			return;
 		}

src/view/view.js

@@ -24,7 +24,6 @@ import CompositionObserver from './observer/compositionobserver';
 import InputObserver from './observer/inputobserver';
 
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
-import log from '@ckeditor/ckeditor5-utils/src/log';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import { scrollViewportToShowTarget } from '@ckeditor/ckeditor5-utils/src/dom/scroll';
 import { injectUiElementHandling } from './uielement';
@@ -398,14 +397,11 @@ export default class View {
 				this.domConverter.focus( editable );
 				this.forceRender();
 			} else {
-				/**
-				 * Before focusing view document, selection should be placed inside one of the view's editables.
-				 * Normally its selection will be converted from model document (which have default selection), but
-				 * when using view document on its own, we need to manually place selection before focusing it.
-				 *
-				 * @error view-focus-no-selection
-				 */
-				log.warn( 'view-focus-no-selection: There is no selection in any editable to focus.' );
+				// Before focusing view document, selection should be placed inside one of the view's editables.
+				// Normally its selection will be converted from model document (which have default selection), but
+				// when using view document on its own, we need to manually place selection before focusing it.
+				//
+				// @if CK_DEBUG // console.warn( 'There is no selection in any editable to focus.' );
 			}
 		}
 	}

tests/conversion/conversion.js

@@ -81,7 +81,7 @@ describe( 'Conversion', () => {
 
 		it( 'should be chainable', () => {
 			const helpers = conversion.for( 'upcast' );
-			const addResult = helpers.add( () => { } );
+			const addResult = helpers.add( () => {} );
 
 			expect( addResult ).to.equal( helpers );
 		} );

tests/conversion/downcastdispatcher.js

@@ -392,8 +392,8 @@ describe( 'DowncastDispatcher', () => {
 			const viewFigure = new ViewContainerElement( 'figure', null, viewCaption );
 
 			// Create custom highlight handler mock.
-			viewFigure._setCustomProperty( 'addHighlight', () => { } );
-			viewFigure._setCustomProperty( 'removeHighlight', () => { } );
+			viewFigure._setCustomProperty( 'addHighlight', () => {} );
+			viewFigure._setCustomProperty( 'removeHighlight', () => {} );
 
 			// Create mapper mock.
 			dispatcher.conversionApi.mapper = {

tests/conversion/downcasthelpers.js

@@ -3,6 +3,8 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
+/* globals console */
+
 import EditingController from '../../src/controller/editingcontroller';
 
 import Model from '../../src/model/model';
@@ -15,7 +17,6 @@ import ViewContainerElement from '../../src/view/containerelement';
 import ViewUIElement from '../../src/view/uielement';
 import ViewText from '../../src/view/text';
 
-import log from '@ckeditor/ckeditor5-utils/src/log';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 
 import DowncastHelpers, {
@@ -32,6 +33,7 @@ import { stringify as stringifyView } from '../../src/dev-utils/view';
 import View from '../../src/view/view';
 import createViewRoot from '../view/_utils/createroot';
 import { setData as setModelData } from '../../src/dev-utils/model';
+import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
 
 describe( 'DowncastHelpers', () => {
 	let model, modelRoot, viewRoot, downcastHelpers, controller;
@@ -614,7 +616,7 @@ describe( 'DowncastHelpers', () => {
 
 		// #1587
 		it( 'config.view and config.model as strings in generic conversion (elements only)', () => {
-			const logStub = testUtils.sinon.stub( log, 'warn' );
+			const consoleWarnStub = testUtils.sinon.stub( console, 'warn' );
 
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 
@@ -626,7 +628,7 @@ describe( 'DowncastHelpers', () => {
 			} );
 
 			expectResult( '<p test="1"></p><p test="2"></p>' );
-			expect( logStub.callCount ).to.equal( 0 );
+			expect( consoleWarnStub.callCount ).to.equal( 0 );
 
 			model.change( writer => {
 				writer.removeAttribute( 'test', modelRoot.getChild( 1 ) );
@@ -637,29 +639,19 @@ describe( 'DowncastHelpers', () => {
 
 		// #1587
 		it( 'config.view and config.model as strings in generic conversion (elements + text)', () => {
-			const logStub = testUtils.sinon.stub( log, 'warn' );
-
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 
 			downcastHelpers.attributeToAttribute( { model: 'test', view: 'test' } );
 
-			model.change( writer => {
-				writer.insertElement( 'paragraph', modelRoot, 0 );
-				writer.insertElement( 'paragraph', { test: '1' }, modelRoot, 1 );
-
-				writer.insertText( 'Foo', { test: '2' }, modelRoot.getChild( 0 ), 0 );
-				writer.insertText( 'Bar', { test: '3' }, modelRoot.getChild( 1 ), 0 );
-			} );
-
-			expectResult( '<p>Foo</p><p test="1">Bar</p>' );
-			expect( logStub.callCount ).to.equal( 2 );
-			expect( logStub.alwaysCalledWithMatch( 'conversion-attribute-to-attribute-on-text' ) ).to.true;
-
-			model.change( writer => {
-				writer.removeAttribute( 'test', modelRoot.getChild( 1 ) );
-			} );
+			expectToThrowCKEditorError( () => {
+				model.change( writer => {
+					writer.insertElement( 'paragraph', modelRoot, 0 );
+					writer.insertElement( 'paragraph', { test: '1' }, modelRoot, 1 );
 
-			expectResult( '<p>Foo</p><p>Bar</p>' );
+					writer.insertText( 'Foo', { test: '2' }, modelRoot.getChild( 0 ), 0 );
+					writer.insertText( 'Bar', { test: '3' }, modelRoot.getChild( 1 ), 0 );
+				} );
+			}, /^conversion-attribute-to-attribute-on-text/ );
 		} );
 
 		it( 'should convert attribute insert/change/remove on a model node', () => {

tests/model/document.js

@@ -44,7 +44,7 @@ describe( 'Document', () => {
 				baseVersion: 0,
 				isDocumentOperation: true,
 				_execute: sinon.stub().returns( data ),
-				_validate: () => { }
+				_validate: () => {}
 			};
 
 			batch = new Batch();
@@ -82,7 +82,7 @@ describe( 'Document', () => {
 			const operation = {
 				baseVersion: 1,
 				isDocumentOperation: true,
-				_execute: () => { }
+				_execute: () => {}
 			};
 
 			expectToThrowCKEditorError( () => {