From 938c1d90be98d78306cc5c7a26226dffac39cd05 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 4 May 2021 17:11:20 +0200 Subject: [PATCH 1/4] Docs: Fixed some outdated docs in `model.TreeWalker`. --- packages/ckeditor5-engine/src/model/treewalker.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-engine/src/model/treewalker.js b/packages/ckeditor5-engine/src/model/treewalker.js index 7937c03462b..18f3221e14f 100644 --- a/packages/ckeditor5-engine/src/model/treewalker.js +++ b/packages/ckeditor5-engine/src/model/treewalker.js @@ -382,10 +382,9 @@ function formatReturnValue( type, item, previousPosition, nextPosition, length ) /** * Type of the step made by {@link module:engine/model/treewalker~TreeWalker}. * Possible values: `'elementStart'` if walker is at the beginning of a node, `'elementEnd'` if walker is at the end of node, - * `'character'` if walker traversed over a character, or `'text'` if walker traversed over multiple characters (available in - * character merging mode, see {@link module:engine/model/treewalker~TreeWalker#constructor}). + * or `'text'` if walker traversed over text. * - * @typedef {'elementStart'|'elementEnd'|'character'|'text'} module:engine/model/treewalker~TreeWalkerValueType + * @typedef {'elementStart'|'elementEnd'|'text'} module:engine/model/treewalker~TreeWalkerValueType */ /** @@ -404,7 +403,7 @@ function formatReturnValue( type, item, previousPosition, nextPosition, length ) * the position after the item. * * Backward iteration: For `'elementEnd'` it is last position inside element. For all other types it is the position * before the item. - * @property {Number} [length] Length of the item. For `'elementStart'` and `'character'` it is 1. For `'text'` it is + * @property {Number} [length] Length of the item. For `'elementStart'` it is 1. For `'text'` it is * the length of the text. For `'elementEnd'` it is `undefined`. */ From 220232a327217c981542aa426ce1b1659ae95fa2 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 4 May 2021 17:24:45 +0200 Subject: [PATCH 2/4] Other (engine): In marker-to-data conversion, attributes for marker boundaries will be used every time the marker starts or ends before or after a model elements, instead only where text is not allowed by model schema. --- .../src/conversion/downcasthelpers.js | 30 +++++------ .../tests/conversion/downcasthelpers.js | 54 +++++++++++++++++++ 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js index acfd26b880e..9f147faad44 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js @@ -405,9 +405,8 @@ export default class DowncastHelpers extends ConversionHelpers { * * This conversion creates a representation for model marker boundaries in the view: * - * * If the marker boundary is at a position where text nodes are allowed, then a view element with the specified tag name - * and `name` attribute is added at this position. - * * In other cases, a specified attribute is set on a view element that is before or after the marker boundary. + * * If the marker boundary is before or after a model element, a view attribute is set on a corresponding view element. + * * In other cases, a view element with the specified tag name is inserted at corresponding view position. * * Typically, marker names use the `group:uniqueId:otherData` convention. For example: `comment:e34zfk9k2n459df53sjl34:zx32c`. * The default configuration for this conversion is that the first part is the `group` part and the rest of @@ -945,32 +944,31 @@ function insertMarkerData( viewCreator ) { // Helper function for `insertMarkerData()` that marks a marker boundary at the beginning or end of given `range`. function handleMarkerBoundary( range, isStart, conversionApi, data, viewMarkerData ) { const modelPosition = isStart ? range.start : range.end; - const canInsertElement = conversionApi.schema.checkChild( modelPosition, '$text' ); + const elementAfter = modelPosition.nodeAfter && modelPosition.nodeAfter.is( 'element' ) ? modelPosition.nodeAfter : null; + const elementBefore = modelPosition.nodeBefore && modelPosition.nodeBefore.is( 'element' ) ? modelPosition.nodeBefore : null; - if ( canInsertElement ) { - const viewPosition = conversionApi.mapper.toViewPosition( modelPosition ); - - insertMarkerAsElement( viewPosition, isStart, conversionApi, data, viewMarkerData ); - } else { + if ( elementAfter || elementBefore ) { let modelElement; let isBefore; // If possible, we want to add `data-group-start-before` and `data-group-end-after` attributes. - // Below `if` is constructed in a way that will favor adding these attributes. - // - // Also, I assume that there will be always an element either after or before the position. - // If not, then it is a case when we are not in a position where text is allowed and also there are no elements around... - if ( isStart && modelPosition.nodeAfter || !isStart && !modelPosition.nodeBefore ) { - modelElement = modelPosition.nodeAfter; + if ( isStart && elementAfter || !isStart && !elementBefore ) { + // `data-group-start-before`, `data-group-end-after`. + modelElement = elementAfter; isBefore = true; } else { - modelElement = modelPosition.nodeBefore; + // `data-group-end-before`, `data-group-start-after`. + modelElement = elementBefore; isBefore = false; } const viewElement = conversionApi.mapper.toViewElement( modelElement ); insertMarkerAsAttribute( viewElement, isStart, isBefore, conversionApi, data, viewMarkerData ); + } else { + const viewPosition = conversionApi.mapper.toViewPosition( modelPosition ); + + insertMarkerAsElement( viewPosition, isStart, conversionApi, data, viewMarkerData ); } } diff --git a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js index 86cbbbe6bd5..746f97624ce 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js @@ -6,6 +6,7 @@ /* globals console */ import EditingController from '../../src/controller/editingcontroller'; +import DataController from '../../src/controller/datacontroller'; import Model from '../../src/model/model'; import ModelElement from '../../src/model/element'; @@ -2468,6 +2469,59 @@ describe( 'DowncastHelpers', () => { expectResult( '

Foo

' ); } ); + it( 'default conversion, document fragment, text', () => { + const dataController = new DataController( model, new StylesProcessor() ); + downcastHelpers = new DowncastHelpers( [ dataController.downcastDispatcher ] ); + + downcastHelpers.markerToData( { model: 'group' } ); + + let modelDocumentFragment; + + model.change( writer => { + modelDocumentFragment = writer.createDocumentFragment(); + + writer.insertText( 'foobar', [], modelDocumentFragment, 0 ); + + const range = writer.createRange( + writer.createPositionFromPath( modelDocumentFragment, [ 2 ] ), + writer.createPositionFromPath( modelDocumentFragment, [ 5 ] ) + ); + + modelDocumentFragment.markers.set( 'group:foo:bar', range ); + } ); + + const expectedResult = 'foobar'; + + expect( dataController.stringify( modelDocumentFragment ) ).to.equal( expectedResult ); + } ); + + it( 'default conversion, document fragment, element', () => { + const dataController = new DataController( model, new StylesProcessor() ); + downcastHelpers = new DowncastHelpers( [ dataController.downcastDispatcher ] ); + + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + downcastHelpers.markerToData( { model: 'group' } ); + + let modelDocumentFragment; + + model.change( writer => { + modelDocumentFragment = writer.createDocumentFragment(); + + writer.insertElement( 'paragraph', [], modelDocumentFragment, 0 ); + + const range = writer.createRange( + writer.createPositionFromPath( modelDocumentFragment, [ 0 ] ), + writer.createPositionFromPath( modelDocumentFragment, [ 1 ] ) + ); + + modelDocumentFragment.markers.set( 'group:foo:bar', range ); + } ); + + const expectedResult = '

 

'; + + expect( dataController.stringify( modelDocumentFragment ) ).to.equal( expectedResult ); + } ); + it( 'conversion callback, mixed, multiple markers, name', () => { const customData = { foo: 'bar', From aa5c6b4531fb52b94b027e689f4bbc911f397603 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 5 May 2021 12:11:58 +0200 Subject: [PATCH 3/4] Updated code comment. --- packages/ckeditor5-engine/src/conversion/downcasthelpers.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js index 9f147faad44..78c0e45cc6f 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js @@ -953,11 +953,13 @@ function handleMarkerBoundary( range, isStart, conversionApi, data, viewMarkerDa // If possible, we want to add `data-group-start-before` and `data-group-end-after` attributes. if ( isStart && elementAfter || !isStart && !elementBefore ) { - // `data-group-start-before`, `data-group-end-after`. + // [ -> data-group-start-before elementAfter + // <$text/>] -> data-group-end-before elementAfter modelElement = elementAfter; isBefore = true; } else { - // `data-group-end-before`, `data-group-start-after`. + // [<$text/> -> data-group-start-after elementBefore + // ] -> data-group-end-after elementBefore modelElement = elementBefore; isBefore = false; } From 425b6c9fb64ebd3a44b675b686f78c55d3d8604a Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 5 May 2021 12:34:19 +0200 Subject: [PATCH 4/4] Updated code comment. --- .../ckeditor5-engine/src/conversion/downcasthelpers.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js index 78c0e45cc6f..32886a6537e 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js @@ -953,13 +953,13 @@ function handleMarkerBoundary( range, isStart, conversionApi, data, viewMarkerDa // If possible, we want to add `data-group-start-before` and `data-group-end-after` attributes. if ( isStart && elementAfter || !isStart && !elementBefore ) { - // [ -> data-group-start-before elementAfter - // <$text/>] -> data-group-end-before elementAfter + // [... -> ... + // ] -> modelElement = elementAfter; isBefore = true; } else { - // [<$text/> -> data-group-start-after elementBefore - // ] -> data-group-end-after elementBefore + // ...] -> ... + // [ -> modelElement = elementBefore; isBefore = false; }