diff --git a/packages/ckeditor5-engine/src/model/documentselection.ts b/packages/ckeditor5-engine/src/model/documentselection.ts index c1d9cebb884..5e2dc2ee5fb 100644 --- a/packages/ckeditor5-engine/src/model/documentselection.ts +++ b/packages/ckeditor5-engine/src/model/documentselection.ts @@ -1135,8 +1135,10 @@ class LiveSelection extends Selection { // ...look for a first character node in that range and take attributes from it. for ( const value of range ) { - // If the item is an object, we don't want to get attributes from its children. + // If the item is an object, we don't want to get attributes from its children... if ( value.item.is( 'element' ) && schema.isObject( value.item ) ) { + // ...but collect attributes from inline object. + attrs = getTextAttributes( value.item, schema ); break; } @@ -1237,7 +1239,10 @@ function getTextAttributes( node: Item | null, schema: Schema ): Iterable<[strin // Collect all attributes that can be applied to the text node. for ( const [ key, value ] of node.getAttributes() ) { - if ( schema.checkAttribute( '$text', key ) ) { + if ( + schema.checkAttribute( '$text', key ) && + schema.getAttributeProperties( key ).copyFromObject !== false + ) { attributes.push( [ key, value ] ); } } diff --git a/packages/ckeditor5-engine/src/model/schema.ts b/packages/ckeditor5-engine/src/model/schema.ts index 1a18bbbae4b..a6df5ad2905 100644 --- a/packages/ckeditor5-engine/src/model/schema.ts +++ b/packages/ckeditor5-engine/src/model/schema.ts @@ -1764,6 +1764,18 @@ export interface AttributeProperties { */ copyOnEnter?: boolean; + /** + * Indicates that given attribute should be preserved while replacing the element. + */ + copyOnReplace?: boolean; + + /** + * Indicates that given text attribute should be copied from an inline object to the next inserted inline content. + * + * @default true + */ + copyFromObject?: boolean; + [ name: string ]: unknown; } diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index ee9c9305dc3..f21a8c0b78a 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -142,9 +142,9 @@ export default class DomConverter { private readonly _rawContentElementMatcher = new Matcher(); /** - * A set of encountered raw content DOM nodes. It is used for preventing left trimming of the following text node. + * Matcher for inline object view elements. This is an extension of a simple {@link #inlineObjectElements} array of element names. */ - private readonly _encounteredRawContentDomNodes = new WeakSet(); + private readonly _inlineObjectElementMatcher = new Matcher(); /** * Creates a DOM converter. @@ -661,82 +661,28 @@ export default class DomConverter { skipComments?: boolean; } = {} ): ViewNode | ViewDocumentFragment | null { - if ( this.isBlockFiller( domNode ) ) { - return null; - } + const inlineNodes: Array = []; + const generator = this._domToView( domNode, options, inlineNodes ); - // When node is inside a UIElement or a RawElement return that parent as it's view representation. - const hostElement = this.getHostViewElement( domNode ); - - if ( hostElement ) { - return hostElement; - } + // Get the first yielded value or a returned value. + const node = generator.next().value; - if ( isComment( domNode ) && options.skipComments ) { + if ( !node ) { return null; } - if ( isText( domNode ) ) { - if ( isInlineFiller( domNode ) ) { - return null; - } else { - const textData = this._processDataFromDomText( domNode ); - - return textData === '' ? null : new ViewText( this.document, textData ); - } - } else { - if ( this.mapDomToView( domNode as ( DomElement | DomDocumentFragment ) ) ) { - return this.mapDomToView( domNode as ( DomElement | DomDocumentFragment ) )!; - } - - let viewElement; - - if ( this.isDocumentFragment( domNode ) ) { - // Create view document fragment. - viewElement = new ViewDocumentFragment( this.document ); - - if ( options.bind ) { - this.bindDocumentFragments( domNode, viewElement ); - } - } else { - // Create view element. - viewElement = this._createViewElement( domNode, options ); - - if ( options.bind ) { - this.bindElements( domNode as DomElement, viewElement ); - } - - // Copy element's attributes. - const attrs = ( domNode as DomElement ).attributes; - - if ( attrs ) { - for ( let l = attrs.length, i = 0; i < l; i++ ) { - viewElement._setAttribute( attrs[ i ].name, attrs[ i ].value ); - } - } - - // Treat this element's content as a raw data if it was registered as such. - // Comment node is also treated as an element with raw data. - if ( this._isViewElementWithRawContent( viewElement, options ) || isComment( domNode ) ) { - const rawContent = isComment( domNode ) ? domNode.data : ( domNode as DomElement ).innerHTML; - - viewElement._setCustomProperty( '$rawContent', rawContent ); + // Trigger children handling. + generator.next(); - // Store a DOM node to prevent left trimming of the following text node. - this._encounteredRawContentDomNodes.add( domNode ); + // Whitespace cleaning. + this._processDomInlineNodes( null, inlineNodes, options ); - return viewElement; - } - } - - if ( options.withChildren !== false ) { - for ( const child of this.domChildrenToView( domNode as DomElement, options ) ) { - viewElement._appendChild( child ); - } - } - - return viewElement; + // Text not got trimmed to an empty string so there is no result node. + if ( node.is( '$text' ) && node.data.length == 0 ) { + return null; } + + return node; } /** @@ -746,18 +692,36 @@ export default class DomConverter { * * @param domElement Parent DOM element. * @param options See {@link module:engine/view/domconverter~DomConverter#domToView} options parameter. + * @param inlineNodes An array that will be populated with inline nodes. It's used internally for whitespace processing. * @returns View nodes. */ - public* domChildrenToView( domElement: DomElement, options: Parameters[ 1 ] ): - IterableIterator { + public* domChildrenToView( + domElement: DomElement, + options: Parameters[ 1 ] = {}, + inlineNodes: Array = [] + ): IterableIterator { for ( let i = 0; i < domElement.childNodes.length; i++ ) { const domChild = domElement.childNodes[ i ]; - const viewChild = this.domToView( domChild, options ) as ViewNode | null; + const generator = this._domToView( domChild, options, inlineNodes ); + + // Get the first yielded value or a returned value. + const viewChild = generator.next().value as ViewNode | null; if ( viewChild !== null ) { + // Whitespace cleaning before entering a block element (between block elements). + if ( this._isBlockViewElement( viewChild ) ) { + this._processDomInlineNodes( domElement, inlineNodes, options ); + } + yield viewChild; + + // Trigger children handling. + generator.next(); } } + + // Whitespace cleaning before leaving a block element (content of block element). + this._processDomInlineNodes( domElement, inlineNodes, options ); } /** @@ -1235,6 +1199,20 @@ export default class DomConverter { this._rawContentElementMatcher.add( pattern ); } + /** + * Registers a {@link module:engine/view/matcher~MatcherPattern} for inline object view elements. + * + * This is affecting how {@link module:engine/view/domconverter~DomConverter#domToView} and + * {@link module:engine/view/domconverter~DomConverter#domChildrenToView} process DOM nodes. + * + * This is an extension of a simple {@link #inlineObjectElements} array of element names. + * + * @param pattern Pattern matching a view element which should be treated as an inline object. + */ + public registerInlineObjectMatcher( pattern: MatcherPattern ): void { + this._inlineObjectElementMatcher.add( pattern ); + } + /** * Returns the block {@link module:engine/view/filler filler} node based on the current {@link #blockFillerMode} setting. */ @@ -1280,6 +1258,245 @@ export default class DomConverter { return true; } + /** + * Internal generator for {@link #domToView}. Also used by {@link #domChildrenToView}. + * Separates DOM nodes conversion from whitespaces processing. + * + * @param domNode DOM node or document fragment to transform. + * @param inlineNodes An array of recently encountered inline nodes truncated to the block element boundaries. + * Used later to process whitespaces. + */ + private* _domToView( + domNode: DomNode, + options: { + bind?: boolean; + withChildren?: boolean; + keepOriginalCase?: boolean; + skipComments?: boolean; + }, + inlineNodes: Array + ): IterableIterator { + if ( this.isBlockFiller( domNode ) ) { + return null; + } + + // When node is inside a UIElement or a RawElement return that parent as it's view representation. + const hostElement = this.getHostViewElement( domNode ); + + if ( hostElement ) { + return hostElement; + } + + if ( isComment( domNode ) && options.skipComments ) { + return null; + } + + if ( isText( domNode ) ) { + if ( isInlineFiller( domNode ) ) { + return null; + } else { + const textData = domNode.data; + + if ( textData === '' ) { + return null; + } + + const textNode = new ViewText( this.document, textData ); + + inlineNodes.push( textNode ); + + return textNode; + } + } else { + let viewElement = this.mapDomToView( domNode as ( DomElement | DomDocumentFragment ) ); + + if ( viewElement ) { + if ( this._isInlineObjectElement( viewElement ) ) { + inlineNodes.push( viewElement ); + } + + return viewElement; + } + + if ( this.isDocumentFragment( domNode ) ) { + // Create view document fragment. + viewElement = new ViewDocumentFragment( this.document ); + + if ( options.bind ) { + this.bindDocumentFragments( domNode, viewElement ); + } + } else { + // Create view element. + viewElement = this._createViewElement( domNode, options ); + + if ( options.bind ) { + this.bindElements( domNode as DomElement, viewElement ); + } + + // Copy element's attributes. + const attrs = ( domNode as DomElement ).attributes; + + if ( attrs ) { + for ( let l = attrs.length, i = 0; i < l; i++ ) { + viewElement._setAttribute( attrs[ i ].name, attrs[ i ].value ); + } + } + + // Treat this element's content as a raw data if it was registered as such. + if ( this._isViewElementWithRawContent( viewElement, options ) ) { + viewElement._setCustomProperty( '$rawContent', ( domNode as DomElement ).innerHTML ); + + if ( !this._isBlockViewElement( viewElement ) ) { + inlineNodes.push( viewElement ); + } + + return viewElement; + } + + // Comment node is also treated as an element with raw data. + if ( isComment( domNode ) ) { + viewElement._setCustomProperty( '$rawContent', domNode.data ); + + return viewElement; + } + } + + // Yield the element first so the flow of nested inline nodes is not reversed inside elements. + yield viewElement; + + const nestedInlineNodes: Array = []; + + if ( options.withChildren !== false ) { + for ( const child of this.domChildrenToView( domNode as DomElement, options, nestedInlineNodes ) ) { + viewElement._appendChild( child ); + } + } + + // Check if this is an inline object after processing child nodes so matcher + // for inline objects can verify if the element is empty. + if ( this._isInlineObjectElement( viewElement ) ) { + inlineNodes.push( viewElement ); + } else { + // It's an inline element that is not an object (like , ) or a block element. + for ( const inlineNode of nestedInlineNodes ) { + inlineNodes.push( inlineNode ); + } + } + } + } + + /** + * Internal helper that walks the list of inline view nodes already generated from DOM nodes + * and handles whitespaces and NBSPs. + * + * @param domParent The DOM parent of the given inline nodes. This should be a document fragment or + * a block element to whitespace processing start cleaning. + * @param inlineNodes An array of recently encountered inline nodes truncated to the block element boundaries. + */ + private _processDomInlineNodes( + domParent: DomElement | null, + inlineNodes: Array, + options: { withChildren?: boolean } + ): void { + if ( !inlineNodes.length ) { + return; + } + + // Process text nodes only after reaching a block or document fragment, + // do not alter whitespaces while processing an inline element like or . + if ( domParent && !this.isDocumentFragment( domParent ) && !this._isBlockDomElement( domParent ) ) { + return; + } + + let prevNodeEndsWithSpace = false; + + for ( let i = 0; i < inlineNodes.length; i++ ) { + const node = inlineNodes[ i ]; + + if ( !node.is( '$text' ) ) { + prevNodeEndsWithSpace = false; + continue; + } + + let data: string; + let nodeEndsWithSpace: boolean = false; + + if ( _hasViewParentOfType( node, this.preElements ) ) { + data = getDataWithoutFiller( node.data ); + } else { + // Change all consecutive whitespace characters (from the [ \n\t\r] set – + // see https://github.com/ckeditor/ckeditor5-engine/issues/822#issuecomment-311670249) to a single space character. + // That's how multiple whitespaces are treated when rendered, so we normalize those whitespaces. + // We're replacing 1+ (and not 2+) to also normalize singular \n\t\r characters (#822). + data = node.data.replace( /[ \n\t\r]{1,}/g, ' ' ); + nodeEndsWithSpace = /[^\S\u00A0]/.test( data.charAt( data.length - 1 ) ); + + const prevNode = i > 0 ? inlineNodes[ i - 1 ] : null; + const nextNode = i + 1 < inlineNodes.length ? inlineNodes[ i + 1 ] : null; + + const shouldLeftTrim = !prevNode || prevNode.is( 'element' ) && prevNode.name == 'br' || prevNodeEndsWithSpace; + const shouldRightTrim = nextNode ? false : !startsWithFiller( node.data ); + + // Do not try to clear whitespaces if this is flat mapping for the purpose of mutation observer and differ in rendering. + if ( options.withChildren !== false ) { + // If the previous dom text node does not exist or it ends by whitespace character, remove space character from the + // beginning of this text node. Such space character is treated as a whitespace. + if ( shouldLeftTrim ) { + data = data.replace( /^ /, '' ); + } + + // If the next text node does not exist remove space character from the end of this text node. + if ( shouldRightTrim ) { + data = data.replace( / $/, '' ); + } + } + + // At the beginning and end of a block element, Firefox inserts normal space +
instead of non-breaking space. + // This means that the text node starts/end with normal space instead of non-breaking space. + // This causes a problem because the normal space would be removed in `.replace` calls above. To prevent that, + // the inline filler is removed only after the data is initially processed (by the `.replace` above). See ckeditor5#692. + data = getDataWithoutFiller( data ); + + // At this point we should have removed all whitespaces from DOM text data. + // + // Now, We will reverse the process that happens in `_processDataFromViewText`. + // + // We have to change   chars, that were in DOM text data because of rendering reasons, to spaces. + // First, change all ` \u00A0` pairs (space +  ) to two spaces. DOM converter changes two spaces from model/view to + // ` \u00A0` to ensure proper rendering. Since here we convert back, we recognize those pairs and change them back to ` `. + data = data.replace( / \u00A0/g, ' ' ); + + const isNextNodeInlineObjectElement = nextNode && nextNode.is( 'element' ) && nextNode.name != 'br'; + const isNextNodeStartingWithSpace = nextNode && nextNode.is( '$text' ) && nextNode.data.charAt( 0 ) == ' '; + + // Then, let's change the last nbsp to a space. + if ( /[ \u00A0]\u00A0$/.test( data ) || !nextNode || isNextNodeInlineObjectElement || isNextNodeStartingWithSpace ) { + data = data.replace( /\u00A0$/, ' ' ); + } + + // Then, change   character that is at the beginning of the text node to space character. + // We do that replacement only if this is the first node or the previous node ends on whitespace character. + if ( shouldLeftTrim || prevNode && prevNode.is( 'element' ) && prevNode.name != 'br' ) { + data = data.replace( /^\u00A0/, ' ' ); + } + } + + // At this point, all whitespaces should be removed and all   created for rendering reasons should be + // changed to normal space. All left   are   inserted intentionally. + + if ( data.length == 0 && node.parent ) { + node._remove(); + inlineNodes.splice( i, 1 ); + i--; + } else { + node._data = data; + prevNodeEndsWithSpace = nodeEndsWithSpace; + } + } + + inlineNodes.length = 0; + } + /** * Takes text data from a given {@link module:engine/view/text~Text#data} and processes it so * it is correctly displayed in the DOM. @@ -1355,122 +1572,6 @@ export default class DomConverter { return data.charAt( data.length - 1 ) == ' '; } - /** - * Takes text data from native `Text` node and processes it to a correct {@link module:engine/view/text~Text view text node} data. - * - * Following changes are done: - * - * * multiple whitespaces are replaced to a single space, - * * space at the beginning of a text node is removed if it is the first text node in its container - * element or if the previous text node ends with a space character, - * * space at the end of the text node is removed if there are two spaces at the end of a node or if next node - * starts with a space or if it is the last text node in its container - * * nbsps are converted to spaces. - * - * @param node DOM text node to process. - * @returns Processed data. - */ - private _processDataFromDomText( node: DomText ): string { - let data = node.data; - - if ( _hasDomParentOfType( node, this.preElements ) ) { - return getDataWithoutFiller( node ); - } - - // Change all consecutive whitespace characters (from the [ \n\t\r] set – - // see https://github.com/ckeditor/ckeditor5-engine/issues/822#issuecomment-311670249) to a single space character. - // That's how multiple whitespaces are treated when rendered, so we normalize those whitespaces. - // We're replacing 1+ (and not 2+) to also normalize singular \n\t\r characters (#822). - data = data.replace( /[ \n\t\r]{1,}/g, ' ' ); - - const prevNode = this._getTouchingInlineDomNode( node, false ); - const nextNode = this._getTouchingInlineDomNode( node, true ); - - const shouldLeftTrim = this._checkShouldLeftTrimDomText( node, prevNode ); - const shouldRightTrim = this._checkShouldRightTrimDomText( node, nextNode ); - - // If the previous dom text node does not exist or it ends by whitespace character, remove space character from the beginning - // of this text node. Such space character is treated as a whitespace. - if ( shouldLeftTrim ) { - data = data.replace( /^ /, '' ); - } - - // If the next text node does not exist remove space character from the end of this text node. - if ( shouldRightTrim ) { - data = data.replace( / $/, '' ); - } - - // At the beginning and end of a block element, Firefox inserts normal space +
instead of non-breaking space. - // This means that the text node starts/end with normal space instead of non-breaking space. - // This causes a problem because the normal space would be removed in `.replace` calls above. To prevent that, - // the inline filler is removed only after the data is initially processed (by the `.replace` above). See ckeditor5#692. - data = getDataWithoutFiller( new Text( data ) ); - - // At this point we should have removed all whitespaces from DOM text data. - // - // Now, We will reverse the process that happens in `_processDataFromViewText`. - // - // We have to change   chars, that were in DOM text data because of rendering reasons, to spaces. - // First, change all ` \u00A0` pairs (space +  ) to two spaces. DOM converter changes two spaces from model/view to - // ` \u00A0` to ensure proper rendering. Since here we convert back, we recognize those pairs and change them back to ` `. - data = data.replace( / \u00A0/g, ' ' ); - - const isNextNodeInlineObjectElement = nextNode && this.isElement( nextNode ) && nextNode.tagName != 'BR'; - const isNextNodeStartingWithSpace = nextNode && isText( nextNode ) && nextNode.data.charAt( 0 ) == ' '; - - // Then, let's change the last nbsp to a space. - if ( /( |\u00A0)\u00A0$/.test( data ) || !nextNode || isNextNodeInlineObjectElement || isNextNodeStartingWithSpace ) { - data = data.replace( /\u00A0$/, ' ' ); - } - - // Then, change   character that is at the beginning of the text node to space character. - // We do that replacement only if this is the first node or the previous node ends on whitespace character. - if ( shouldLeftTrim || prevNode && this.isElement( prevNode ) && prevNode.tagName != 'BR' ) { - data = data.replace( /^\u00A0/, ' ' ); - } - - // At this point, all whitespaces should be removed and all   created for rendering reasons should be - // changed to normal space. All left   are   inserted intentionally. - return data; - } - - /** - * Helper function which checks if a DOM text node, preceded by the given `prevNode` should - * be trimmed from the left side. - * - * @param prevNode Either DOM text or `
` or one of `#inlineObjectElements`. - */ - private _checkShouldLeftTrimDomText( node: DomText, prevNode: DomNode | null ): boolean { - if ( !prevNode ) { - return true; - } - - if ( this.isElement( prevNode ) ) { - return prevNode.tagName === 'BR'; - } - - // Shouldn't left trim if previous node is a node that was encountered as a raw content node. - if ( this._encounteredRawContentDomNodes.has( node.previousSibling as any ) ) { - return false; - } - - return /[^\S\u00A0]/.test( ( prevNode as DomText ).data.charAt( ( prevNode as DomText ).data.length - 1 ) ); - } - - /** - * Helper function which checks if a DOM text node, succeeded by the given `nextNode` should - * be trimmed from the right side. - * - * @param nextNode Either DOM text or `
` or one of `#inlineObjectElements`. - */ - private _checkShouldRightTrimDomText( node: DomText, nextNode: DomNode | null ): boolean { - if ( nextNode ) { - return false; - } - - return !startsWithFiller( node ); - } - /** * Helper function. For given {@link module:engine/view/text~Text view text node}, it finds previous or next sibling * that is contained in the same container element. If there is no such sibling, `null` is returned. @@ -1486,8 +1587,12 @@ export default class DomConverter { } ); for ( const value of treeWalker ) { + //
found – it works like a block boundary, so do not scan further. + if ( value.item.is( 'element', 'br' ) ) { + return null; + } // Found an inline object (for example an image). - if ( value.item.is( 'element' ) && this.inlineObjectElements.includes( value.item.name ) ) { + else if ( this._isInlineObjectElement( value.item ) ) { return value.item; } // ViewContainerElement is found on a way to next ViewText node, so given `node` was first/last @@ -1495,10 +1600,6 @@ export default class DomConverter { else if ( value.item.is( 'containerElement' ) ) { return null; } - //
found – it works like a block boundary, so do not scan further. - else if ( value.item.is( 'element', 'br' ) ) { - return null; - } // Found a text node in the same container element. else if ( value.item.is( '$textProxy' ) ) { return value.item; @@ -1509,68 +1610,30 @@ export default class DomConverter { } /** - * Helper function. For the given text node, it finds the closest touching node which is either - * a text, `
` or an {@link #inlineObjectElements inline object}. - * - * If no such node is found, `null` is returned. - * - * For instance, in the following DOM structure: - * - * ```html - *

foobar
bom

- * ``` - * - * * `foo` doesn't have its previous touching inline node (`null` is returned), - * * `foo`'s next touching inline node is `bar` - * * `bar`'s next touching inline node is `
` - * - * This method returns text nodes and `
` elements because these types of nodes affect how - * spaces in the given text node need to be converted. + * Returns `true` if a DOM node belongs to {@link #blockElements}. `false` otherwise. */ - private _getTouchingInlineDomNode( node: DomText, getNext: boolean ): DomNode | null { - if ( !node.parentNode ) { - return null; - } - - const stepInto = getNext ? 'firstChild' : 'lastChild'; - const stepOver = getNext ? 'nextSibling' : 'previousSibling'; - - let skipChildren = true; - let returnNode: DomNode | null = node; - - do { - if ( !skipChildren && returnNode[ stepInto ] ) { - returnNode = returnNode[ stepInto ]; - } else if ( returnNode[ stepOver ] ) { - returnNode = returnNode[ stepOver ]; - skipChildren = false; - } else { - returnNode = returnNode.parentNode; - skipChildren = true; - } - - if ( !returnNode || this._isBlockElement( returnNode ) ) { - return null; - } - } while ( - !( isText( returnNode ) || ( returnNode as DomElement ).tagName == 'BR' || this._isInlineObjectElement( returnNode ) ) - ); - - return returnNode; + private _isBlockDomElement( node: DomNode ): boolean { + return this.isElement( node ) && this.blockElements.includes( node.tagName.toLowerCase() ); } /** - * Returns `true` if a DOM node belongs to {@link #blockElements}. `false` otherwise. + * Returns `true` if a view node belongs to {@link #blockElements}. `false` otherwise. */ - private _isBlockElement( node: DomNode ): boolean { - return this.isElement( node ) && this.blockElements.includes( node.tagName.toLowerCase() ); + private _isBlockViewElement( node: ViewNode ): boolean { + return node.is( 'element' ) && this.blockElements.includes( node.name ); } /** * Returns `true` if a DOM node belongs to {@link #inlineObjectElements}. `false` otherwise. */ - private _isInlineObjectElement( node: DomNode ): boolean { - return this.isElement( node ) && this.inlineObjectElements.includes( node.tagName.toLowerCase() ); + private _isInlineObjectElement( node: ViewNode | ViewTextProxy | ViewDocumentFragment ): node is ViewElement { + if ( !node.is( 'element' ) ) { + return false; + } + + return node.name == 'br' || + this.inlineObjectElements.includes( node.name ) || + !!this._inlineObjectElementMatcher.match( node ); } /** @@ -1595,8 +1658,8 @@ export default class DomConverter { * @param viewElement View element to check. * @param options Conversion options. See {@link module:engine/view/domconverter~DomConverter#domToView} options parameter. */ - private _isViewElementWithRawContent( viewElement: ViewElement, options: { withChildren?: boolean } ): boolean { - return options.withChildren !== false && !!this._rawContentElementMatcher.match( viewElement ); + private _isViewElementWithRawContent( viewElement: ViewElement | ViewDocumentFragment, options: { withChildren?: boolean } ): boolean { + return options.withChildren !== false && viewElement.is( 'element' ) && !!this._rawContentElementMatcher.match( viewElement ); } /** @@ -1643,10 +1706,8 @@ export default class DomConverter { * * @returns`true` if such parent exists or `false` if it does not. */ -function _hasDomParentOfType( node: DomNode, types: ReadonlyArray ) { - const parents = getAncestors( node ); - - return parents.some( parent => ( parent as DomElement ).tagName && types.includes( ( parent as DomElement ).tagName.toLowerCase() ) ); +function _hasViewParentOfType( node: ViewNode, types: ReadonlyArray ) { + return node.getAncestors().some( parent => parent.is( 'element' ) && types.includes( parent.name ) ); } /** diff --git a/packages/ckeditor5-engine/src/view/filler.ts b/packages/ckeditor5-engine/src/view/filler.ts index bd32347062c..20f523b2651 100644 --- a/packages/ckeditor5-engine/src/view/filler.ts +++ b/packages/ckeditor5-engine/src/view/filler.ts @@ -98,7 +98,11 @@ export const INLINE_FILLER = '\u2060'.repeat( INLINE_FILLER_LENGTH ); * @param domNode DOM node. * @returns True if the text node starts with the {@link module:engine/view/filler~INLINE_FILLER inline filler}. */ -export function startsWithFiller( domNode: Node ): boolean { +export function startsWithFiller( domNode: Node | string ): boolean { + if ( typeof domNode == 'string' ) { + return domNode.substr( 0, INLINE_FILLER_LENGTH ) === INLINE_FILLER; + } + return isText( domNode ) && ( domNode.data.substr( 0, INLINE_FILLER_LENGTH ) === INLINE_FILLER ); } @@ -129,12 +133,14 @@ export function isInlineFiller( domText: Text ): boolean { * @param domText DOM text node, possible with inline filler. * @returns Data without filler. */ -export function getDataWithoutFiller( domText: Text ): string { +export function getDataWithoutFiller( domText: Text | string ): string { + const data = typeof domText == 'string' ? domText : domText.data; + if ( startsWithFiller( domText ) ) { - return domText.data.slice( INLINE_FILLER_LENGTH ); - } else { - return domText.data; + return data.slice( INLINE_FILLER_LENGTH ); } + + return data; } /** diff --git a/packages/ckeditor5-engine/tests/model/documentselection.js b/packages/ckeditor5-engine/tests/model/documentselection.js index 28653ee27ac..eed57b5a4f8 100644 --- a/packages/ckeditor5-engine/tests/model/documentselection.js +++ b/packages/ckeditor5-engine/tests/model/documentselection.js @@ -1424,7 +1424,8 @@ describe( 'DocumentSelection', () => { describe( 'reads surrounding attributes from inline object elements', () => { beforeEach( () => { model.schema.register( 'imageInline', { - inheritAllFrom: '$inlineObject' + inheritAllFrom: '$inlineObject', + allowAttributes: 'src' } ); } ); @@ -1449,6 +1450,45 @@ describe( 'DocumentSelection', () => { expect( selection.hasAttribute( 'bold' ) ).to.equal( true ); } ); + + it( 'ignores attributes from a node before (copyFromObject === false)', () => { + model.schema.setAttributeProperties( 'bold', { copyFromObject: false } ); + setData( model, '

<$text bold="true">Foo Bar.[]

' ); + + expect( selection.hasAttribute( 'bold' ) ).to.equal( false ); + } ); + + it( 'ignores attributes from a node after with override gravity (copyFromObject === false)', () => { + model.schema.setAttributeProperties( 'bold', { copyFromObject: false } ); + setData( model, '

<$text>Foo Bar.[]

' ); + + const overrideGravityUid = selection._overrideGravity(); + + expect( selection.hasAttribute( 'bold' ) ).to.equal( false ); + + selection._restoreGravity( overrideGravityUid ); + } ); + + it( 'ignores attributes from , even without any text before it (copyFromObject === false)', () => { + model.schema.setAttributeProperties( 'bold', { copyFromObject: false } ); + setData( model, '

[]

' ); + + expect( selection.hasAttribute( 'bold' ) ).to.equal( false ); + } ); + + it( 'inherits attributes from a selected node (only those allowed on text)', () => { + setData( model, '

foo[]bar

' ); + + expect( selection.hasAttribute( 'bold' ) ).to.equal( true ); + expect( selection.hasAttribute( 'src' ) ).to.equal( false ); + } ); + + it( 'ignores attributes from a selected node with copyFromObject flag == false', () => { + model.schema.setAttributeProperties( 'bold', { copyFromObject: false } ); + setData( model, '

foo[]bar

' ); + + expect( selection.hasAttribute( 'bold' ) ).to.equal( false ); + } ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/view/domconverter/rawcontent.js b/packages/ckeditor5-engine/tests/view/domconverter/rawcontent.js index cb265408e5d..19e6220cbf4 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/rawcontent.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/rawcontent.js @@ -203,6 +203,54 @@ describe( 'DOMConverter raw content matcher', () => { } ); describe( 'whitespace trimming', () => { + it( 'should trim whitespaces before or after non inline raw content element', () => { + converter.registerRawContentMatcher( { + name: 'div', + classes: 'raw' + } ); + + const domDiv = createElement( document, 'div', {}, [ + createElement( document, 'p' ), + document.createTextNode( ' ' ), + createElement( document, 'div', { class: 'raw' }, ' abc ' ), + document.createTextNode( ' ' ), + createElement( document, 'p' ) + ] ); + + const viewDiv = converter.domToView( domDiv ); + + expect( viewDiv.childCount ).to.equal( 3 ); + expect( viewDiv.getChild( 0 ).name ).to.equal( 'p' ); + expect( viewDiv.getChild( 1 ).getCustomProperty( '$rawContent' ) ).to.equal( ' abc ' ); + expect( viewDiv.getChild( 2 ).name ).to.equal( 'p' ); + } ); + + it( 'should trim whitespaces before or after non inline raw content element with deeper nesting', () => { + converter.registerRawContentMatcher( { + name: 'div', + classes: 'raw' + } ); + + const domDIv = createElement( document, 'div', {}, [ + createElement( document, 'p' ), + document.createTextNode( ' ' ), + createElement( document, 'div', { class: 'raw' }, [ + createElement( document, 'div', { class: 'raw' }, [ + document.createTextNode( ' abc ' ) + ] ) + ] ), + document.createTextNode( ' ' ), + createElement( document, 'p' ) + ] ); + + const viewDiv = converter.domToView( domDIv ); + + expect( viewDiv.childCount ).to.equal( 3 ); + expect( viewDiv.getChild( 0 ).name ).to.equal( 'p' ); + expect( viewDiv.getChild( 1 ).getCustomProperty( '$rawContent' ) ).to.equal( '
abc
' ); + expect( viewDiv.getChild( 2 ).name ).to.equal( 'p' ); + } ); + it( 'should not trim whitespaces before or after raw content inline element', () => { converter.registerRawContentMatcher( { name: 'span' diff --git a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js index 9bbebe72d36..1e53f1d9280 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js @@ -489,6 +489,192 @@ describe( 'DomConverter – whitespace handling – integration', () => { } } ); } ); + + describe( 'around custom inline objects', () => { + beforeEach( () => { + editor.model.schema.register( 'inlineObject', { inheritAllFrom: '$inlineObject' } ); + + editor.conversion.for( 'upcast' ).elementToElement( { + view: { + name: 'span', + classes: 'foo' + }, + model: 'inlineObject' + } ); + editor.conversion.for( 'downcast' ).elementToElement( { + model: 'inlineObject', + view: ( modelElement, { writer } ) => { + const viewElement = writer.createContainerElement( 'span', { class: 'foo' } ); + + viewElement.getFillerOffset = () => null; + + return viewElement; + } + } ); + + editor.data.htmlProcessor.domConverter.registerInlineObjectMatcher( { + name: 'span', + classes: 'foo' + } ); + } ); + + it( 'white space with text before empty inline object is not ignored', () => { + editor.setData( '

foo

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( 'foo ' ); + + expect( editor.getData() ).to.equal( '

foo

' ); + } ); + + it( 'white space with text after empty inline object is not ignored', () => { + editor.setData( '

foo

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( ' foo' ); + + expect( editor.getData() ).to.equal( '

foo

' ); + } ); + + it( 'white spaces with text around empty inline object are not ignored', () => { + editor.setData( '

foo bar

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( 'foo bar' ); + + expect( editor.getData() ).to.equal( '

foo bar

' ); + } ); + + it( 'white space before empty inline object is ignored', () => { + editor.setData( '

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '' ); + + expect( editor.getData() ).to.equal( '

' ); + } ); + + it( 'white space after empty inline object is ignored', () => { + editor.setData( '

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '' ); + + expect( editor.getData() ).to.equal( '

' ); + } ); + + it( 'white spaces around empty inline object are ignored', () => { + editor.setData( '

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '' ); + + expect( editor.getData() ).to.equal( '

' ); + } ); + + it( 'nbsp before empty inline object is not ignored', () => { + editor.setData( '

 

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( ' ' ); + + expect( editor.getData() ).to.equal( '

 

' ); + } ); + + it( 'nbsp after empty inline object is not ignored', () => { + editor.setData( '

 

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( ' ' ); + + expect( editor.getData() ).to.equal( '

 

' ); + } ); + + it( 'nbsp around empty inline object are not ignored', () => { + editor.setData( '

  

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( ' ' ); + + expect( editor.getData() ).to.equal( '

  

' ); + } ); + + it( 'text+nbsp before empty inline object is not ignored', () => { + editor.setData( '

foo 

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( 'foo ' ); + + expect( editor.getData() ).to.equal( '

foo

' ); + } ); + + it( 'nbsp+text after empty inline object is not ignored', () => { + editor.setData( '

 foo

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( ' foo' ); + + expect( editor.getData() ).to.equal( '

foo

' ); + } ); + + it( 'text+nbsp or nbsp+text around empty inline object are not ignored', () => { + editor.setData( '

foo  bar

' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( 'foo bar' ); + + expect( editor.getData() ).to.equal( '

foo bar

' ); + } ); + } ); + + it( 'in preformatted blocks', () => { + editor.model.schema.register( 'pre', { inheritAllFrom: '$block' } ); + editor.conversion.elementToElement( { model: 'pre', view: 'pre' } ); + + editor.setData( '
    foo\n    bar\n    
' ); + + expect( getData( editor.model, { withoutSelection: true } ) ) + .to.equal( '
    foo\n    bar\n    
' ); + + expect( editor.getData() ).to.equal( '
    foo\n    bar\n    
' ); + } ); + + it( 'in nested blocks', () => { + editor.model.schema.register( 'ul', { inheritAllFrom: '$block', allowIn: 'li' } ); + editor.model.schema.register( 'li', { inheritAllFrom: '$block', allowIn: 'ul' } ); + editor.conversion.elementToElement( { model: 'ul', view: 'ul' } ); + editor.conversion.elementToElement( { model: 'li', view: 'li' } ); + + editor.setData( ` +
    +
  • 1 +
      +
    • 2
    • +
    +
  • +
+ ` ); + + expect( getData( editor.model, { withoutSelection: true } ) ).to.equal( + '
    ' + + '
  • 1' + + '
      ' + + '
    • 2
    • ' + + '
    ' + + '
  • ' + + '
' + ); + + expect( editor.getData() ).to.equal( + '
    ' + + '
  • 1' + + '
      ' + + '
    • 2
    • ' + + '
    ' + + '
  • ' + + '
' + ); + } ); } ); // https://github.com/ckeditor/ckeditor5/issues/1024 diff --git a/packages/ckeditor5-engine/tests/view/filler.js b/packages/ckeditor5-engine/tests/view/filler.js index 925f762165b..6cc3d53cefa 100644 --- a/packages/ckeditor5-engine/tests/view/filler.js +++ b/packages/ckeditor5-engine/tests/view/filler.js @@ -34,6 +34,18 @@ describe( 'filler', () => { expect( startsWithFiller( node ) ).to.be.true; } ); + it( 'should be true for text which contains only filler', () => { + const str = `${ INLINE_FILLER }`; + + expect( startsWithFiller( str ) ).to.be.true; + } ); + + it( 'should be true for text which starts with filler', () => { + const str = `${ INLINE_FILLER }foo`; + + expect( startsWithFiller( str ) ).to.be.true; + } ); + it( 'should be false for element', () => { const node = document.createElement( 'p' ); @@ -75,6 +87,15 @@ describe( 'filler', () => { expect( dataWithoutFiller ).to.equals( 'foo' ); } ); + it( 'should return text without filler', () => { + const str = `${ INLINE_FILLER }foo`; + + const dataWithoutFiller = getDataWithoutFiller( str ); + + expect( dataWithoutFiller.length ).to.equals( 3 ); + expect( dataWithoutFiller ).to.equals( 'foo' ); + } ); + it( 'should return the same data for data without filler', () => { const node = document.createTextNode( 'foo' ); @@ -83,6 +104,15 @@ describe( 'filler', () => { expect( dataWithoutFiller.length ).to.equals( 3 ); expect( dataWithoutFiller ).to.equals( 'foo' ); } ); + + it( 'should return the same data for text without filler', () => { + const node = 'foo'; + + const dataWithoutFiller = getDataWithoutFiller( node ); + + expect( dataWithoutFiller.length ).to.equals( 3 ); + expect( dataWithoutFiller ).to.equals( 'foo' ); + } ); } ); describe( 'isInlineFiller()', () => { diff --git a/packages/ckeditor5-html-support/src/converters.ts b/packages/ckeditor5-html-support/src/converters.ts index a77c14fd6dd..90202a42a3b 100644 --- a/packages/ckeditor5-html-support/src/converters.ts +++ b/packages/ckeditor5-html-support/src/converters.ts @@ -19,7 +19,8 @@ import type { UpcastConversionApi, UpcastDispatcher, UpcastElementEvent, - ViewElement + ViewElement, + Item } from 'ckeditor5/src/engine'; import { toWidget } from 'ckeditor5/src/widget'; import { @@ -100,10 +101,10 @@ export function createObjectView( viewName: string, modelElement: Element, write * @returns Returns a conversion callback. */ export function viewToAttributeInlineConverter( - { view: viewName, model: attributeKey }: DataSchemaInlineElementDefinition, + { view: viewName, model: attributeKey, allowEmpty }: DataSchemaInlineElementDefinition, dataFilter: DataFilter -) { - return ( dispatcher: UpcastDispatcher ): void => { +): ( dispatcher: UpcastDispatcher ) => void { + return dispatcher => { dispatcher.on( `element:${ viewName }`, ( evt, data, conversionApi ) => { let viewAttributes = dataFilter.processViewAttributes( data.viewItem, conversionApi ); @@ -125,19 +126,68 @@ export function viewToAttributeInlineConverter( data = Object.assign( data, conversionApi.convertChildren( data.viewItem, data.modelCursor ) ); } - // Set attribute on each item in range according to the schema. - for ( const node of data.modelRange!.getItems() ) { - if ( conversionApi.schema.checkAttribute( node, attributeKey ) ) { - // Node's children are converted recursively, so node can already include model attribute. - // We want to extend it, not replace. - const nodeAttributes = node.getAttribute( attributeKey ); - const attributesToAdd = mergeViewElementAttributes( viewAttributes, nodeAttributes || {} ); + // Convert empty inline element if allowed and has any attributes. + if ( allowEmpty && data.modelRange!.isCollapsed && Object.keys( viewAttributes ).length ) { + const modelElement = conversionApi.writer.createElement( 'htmlEmptyElement' ); - conversionApi.writer.setAttribute( attributeKey, attributesToAdd, node ); + if ( !conversionApi.safeInsert( modelElement, data.modelCursor ) ) { + return; } + + const parts = conversionApi.getSplitParts( modelElement ); + + data.modelRange = conversionApi.writer.createRange( + data.modelRange!.start, + conversionApi.writer.createPositionAfter( parts[ parts.length - 1 ] ) + ); + + conversionApi.updateConversionResult( modelElement, data ); + setAttributeOnItem( modelElement, viewAttributes, conversionApi ); + + return; + } + + // Set attribute on each item in range according to the schema. + for ( const node of data.modelRange!.getItems() ) { + setAttributeOnItem( node, viewAttributes, conversionApi ); } }, { priority: 'low' } ); }; + + function setAttributeOnItem( node: Item, viewAttributes: GHSViewAttributes, conversionApi: UpcastConversionApi ): void { + if ( conversionApi.schema.checkAttribute( node, attributeKey ) ) { + // Node's children are converted recursively, so node can already include model attribute. + // We want to extend it, not replace. + const nodeAttributes = node.getAttribute( attributeKey ); + const attributesToAdd = mergeViewElementAttributes( viewAttributes, nodeAttributes || {} ); + + conversionApi.writer.setAttribute( attributeKey, attributesToAdd, node ); + } + } +} + +/** + * Conversion helper converting an empty inline model element to an HTML element or widget. + */ +export function emptyInlineModelElementToViewConverter( + { model: attributeKey, view: viewName }: DataSchemaInlineElementDefinition, + asWidget?: boolean +): ElementCreatorFunction { + return ( item, { writer, consumable } ) => { + if ( !item.hasAttribute( attributeKey ) ) { + return null; + } + + const viewElement = writer.createContainerElement( viewName! ); + const attributeValue = item.getAttribute( attributeKey ) as GHSViewAttributes; + + consumable.consume( item, `attribute:${ attributeKey }` ); + setViewAttributes( writer, attributeValue, viewElement ); + + viewElement.getFillerOffset = () => null; + + return asWidget ? toWidget( viewElement, writer ) : viewElement; + }; } /** diff --git a/packages/ckeditor5-html-support/src/datafilter.ts b/packages/ckeditor5-html-support/src/datafilter.ts index 8c6006178d0..de681af2e7a 100644 --- a/packages/ckeditor5-html-support/src/datafilter.ts +++ b/packages/ckeditor5-html-support/src/datafilter.ts @@ -15,7 +15,8 @@ import { type UpcastConversionApi, type ViewElement, type MatchResult, - type ViewConsumable + type ViewConsumable, + type MatcherObjectPattern } from 'ckeditor5/src/engine'; import { @@ -33,6 +34,7 @@ import { viewToAttributeInlineConverter, attributeToViewInlineConverter, + emptyInlineModelElementToViewConverter, viewToModelBlockAttributeConverter, modelToViewBlockAttributeConverter @@ -54,8 +56,6 @@ import { isPlainObject, pull as removeItemFromArray } from 'lodash-es'; import '../theme/datafilter.css'; -type MatcherPatternWithName = MatcherPattern & { name?: string }; - /** * Allows to validate elements and element attributes registered by {@link module:html-support/dataschema~DataSchema}. * @@ -172,8 +172,8 @@ export default class DataFilter extends Plugin { * * @param config Configuration of elements that should have their attributes accepted in the editor. */ - public loadAllowedConfig( config: Array ): void { - for ( const pattern of config as Array ) { + public loadAllowedConfig( config: Array ): void { + for ( const pattern of config ) { // MatcherPattern allows omitting `name` to widen the search of elements. // Let's keep it consistent and match every element if a `name` has not been provided. const elementName = pattern.name || /[\s\S]+/; @@ -192,8 +192,8 @@ export default class DataFilter extends Plugin { * * @param config Configuration of elements that should have their attributes rejected from the editor. */ - public loadDisallowedConfig( config: Array ): void { - for ( const pattern of config as Array ) { + public loadDisallowedConfig( config: Array ): void { + for ( const pattern of config ) { // MatcherPattern allows omitting `name` to widen the search of elements. // Let's keep it consistent and match every element if a `name` has not been provided. const elementName = pattern.name || /[\s\S]+/; @@ -208,6 +208,19 @@ export default class DataFilter extends Plugin { } } + /** + * Load a configuration of one or many elements, where when empty should be allowed. + * + * **Note**: It modifies DataSchema so must be loaded before registering filtering rules. + * + * @param config Configuration of elements that should be preserved even if empty. + */ + public loadAllowedEmptyElementsConfig( config: Array ): void { + for ( const elementName of config ) { + this.allowEmptyElement( elementName ); + } + } + /** * Allow the given element in the editor context. * @@ -241,6 +254,24 @@ export default class DataFilter extends Plugin { } } + /** + * Allow the given empty element in the editor context. + * + * This method will only allow elements described by the {@link module:html-support/dataschema~DataSchema} used + * to create data filter. + * + * **Note**: It modifies DataSchema so must be called before registering filtering rules. + * + * @param viewName String or regular expression matching view name. + */ + public allowEmptyElement( viewName: string ): void { + for ( const definition of this._dataSchema.getDefinitionsForView( viewName, true ) ) { + if ( definition.isInline ) { + this._dataSchema.extendInlineElement( { ...definition, allowEmpty: true } ); + } + } + } + /** * Allow the given attributes for view element allowed by {@link #allowElement} method. * @@ -668,6 +699,45 @@ export default class DataFilter extends Plugin { model: attributeKey, view: attributeToViewInlineConverter( definition ) } ); + + if ( !definition.allowEmpty ) { + return; + } + + schema.setAttributeProperties( attributeKey, { copyFromObject: false } ); + + if ( !schema.isRegistered( 'htmlEmptyElement' ) ) { + schema.register( 'htmlEmptyElement', { + inheritAllFrom: '$inlineObject' + } ); + } + + editor.data.htmlProcessor.domConverter.registerInlineObjectMatcher( element => { + // Element must be empty and have any attribute. + if ( + element.name == definition.view && + element.isEmpty && + Array.from( element.getAttributeKeys() ).length + ) { + return { + name: true + }; + } + + return null; + } ); + + conversion.for( 'editingDowncast' ) + .elementToElement( { + model: 'htmlEmptyElement', + view: emptyInlineModelElementToViewConverter( definition, true ) + } ); + + conversion.for( 'dataDowncast' ) + .elementToElement( { + model: 'htmlEmptyElement', + view: emptyInlineModelElementToViewConverter( definition ) + } ); } } @@ -836,11 +906,12 @@ function iterableToObject( iterable: Set, getValue: ( s: string ) => any * @param pattern Pattern to split. * @param attributeName Name of the attribute to split (e.g. 'attributes', 'classes', 'styles'). */ -function splitPattern( pattern: MatcherPatternWithName, attributeName: 'attributes' | 'classes' | 'styles' ): Array { +function splitPattern( pattern: MatcherObjectPattern, attributeName: 'attributes' | 'classes' | 'styles' ): Array { const { name } = pattern; - const attributeValue = ( pattern as any )[ attributeName ]; + const attributeValue = pattern[ attributeName ]; + if ( isPlainObject( attributeValue ) ) { - return Object.entries( attributeValue ).map( + return Object.entries( attributeValue as Record ).map( ( [ key, value ] ) => ( { name, [ attributeName ]: { @@ -865,19 +936,21 @@ function splitPattern( pattern: MatcherPatternWithName, attributeName: 'attribut * Rules are matched in conjunction (AND operation), but we want to have a match if *any* of the rules is matched (OR operation). * By splitting the rules we force the latter effect. */ -function splitRules( rules: MatcherPatternWithName ): Array { - const { name, attributes, classes, styles } = rules as any; - const splittedRules = []; +function splitRules( rules: MatcherObjectPattern ): Array { + const { name, attributes, classes, styles } = rules; + const splitRules = []; if ( attributes ) { - splittedRules.push( ...splitPattern( { name, attributes }, 'attributes' ) ); + splitRules.push( ...splitPattern( { name, attributes }, 'attributes' ) ); } + if ( classes ) { - splittedRules.push( ...splitPattern( { name, classes }, 'classes' ) ); + splitRules.push( ...splitPattern( { name, classes }, 'classes' ) ); } + if ( styles ) { - splittedRules.push( ...splitPattern( { name, styles }, 'styles' ) ); + splitRules.push( ...splitPattern( { name, styles }, 'styles' ) ); } - return splittedRules; + return splitRules; } diff --git a/packages/ckeditor5-html-support/src/dataschema.ts b/packages/ckeditor5-html-support/src/dataschema.ts index 299ea47aa86..2a5daa00943 100644 --- a/packages/ckeditor5-html-support/src/dataschema.ts +++ b/packages/ckeditor5-html-support/src/dataschema.ts @@ -302,4 +302,9 @@ export interface DataSchemaInlineElementDefinition extends DataSchemaDefinition * in the `htmlTbodyAttributes` model attribute of the `table` model element. */ appliesToBlock?: boolean | string; + + /** + * Indicates that an element should be preserved even if it has no content. + */ + allowEmpty?: boolean; } diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts index 581e70183d8..86d8db05c1b 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupport.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupport.ts @@ -67,6 +67,10 @@ export default class GeneralHtmlSupport extends Plugin { const editor = this.editor; const dataFilter = editor.plugins.get( DataFilter ); + // Load the allowed empty inline elements' configuration. + // Note that this modifies DataSchema so must be loaded before registering filtering rules. + dataFilter.loadAllowedEmptyElementsConfig( editor.config.get( 'htmlSupport.allowEmpty' ) || [] ); + // Load the filtering configuration. dataFilter.loadAllowedConfig( editor.config.get( 'htmlSupport.allow' ) || [] ); dataFilter.loadDisallowedConfig( editor.config.get( 'htmlSupport.disallow' ) || [] ); diff --git a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts index f789581eaad..4db8b9b3df3 100644 --- a/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts +++ b/packages/ckeditor5-html-support/src/generalhtmlsupportconfig.ts @@ -7,7 +7,7 @@ * @module html-support/generalhtmlsupportconfig */ -import type { MatcherPattern } from 'ckeditor5/src/engine'; +import type { MatcherObjectPattern } from 'ckeditor5/src/engine'; /** * The configuration of the General HTML Support feature. @@ -50,7 +50,7 @@ export interface GeneralHtmlSupportConfig { * ]; * ``` */ - allow?: Array; + allow?: Array; /** * The configuration of disallowed content rules used by General HTML Support. @@ -68,6 +68,16 @@ export interface GeneralHtmlSupportConfig { * ]; * ``` */ - disallow?: Array; + disallow?: Array; + /** + * The configuration of allowed empty inline elements that should not be removed. + * + * Note that you should also add an appropriate entry to {@link #allow} list. + * + * ```ts + * const htmlSupportConfig.allowEmpty = [ 'i', 'span' ]; + * ``` + */ + allowEmpty?: Array; } diff --git a/packages/ckeditor5-html-support/tests/datafilter.js b/packages/ckeditor5-html-support/tests/datafilter.js index cd914b931db..fd911058612 100644 --- a/packages/ckeditor5-html-support/tests/datafilter.js +++ b/packages/ckeditor5-html-support/tests/datafilter.js @@ -1357,6 +1357,225 @@ describe( 'DataFilter', () => { } ); } ); + describe( 'empty inline', () => { + it( 'should allow element', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', classes: true } ); + + editor.setData( '

foo bar

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: 'foo bar', + attributes: { + 1: { + classes: [ 'x' ] + } + } + } ); + + expect( editor.getData() ).to.equal( '

foo bar

' ); + } ); + + it( 'should allow attributes (styles)', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', styles: { color: 'red' } } ); + + editor.setData( '

foo bar

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: 'foo bar', + attributes: { + 1: { + styles: { + color: 'red' + } + } + } + } ); + + expect( editor.getData() ).to.equal( '

foo bar

' ); + } ); + + it( 'should allow attributes (classes)', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', classes: [ 'foobar' ] } ); + + editor.setData( '

foo bar

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: 'foo bar', + attributes: { + 1: { + classes: [ 'foobar' ] + } + } + } ); + + expect( editor.getData() ).to.equal( '

foo bar

' ); + } ); + + it( 'should disallow attributes', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', attributes: true } ); + dataFilter.disallowAttributes( { name: 'i', attributes: { 'data-type': 'hidden' } } ); + + editor.setData( '

foo bar baz

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: '' + + 'foo bar baz' + + '', + attributes: { + 1: { + attributes: { + 'data-type': 'text' + } + }, + 2: { + attributes: { + 'data-x': 'y' + } + } + } + } ); + + expect( editor.getData() ).to.equal( '

foo bar baz

' ); + } ); + + it( 'should disallow attributes (styles)', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', styles: true } ); + dataFilter.disallowAttributes( { name: 'i', styles: { color: 'red' } } ); + + editor.setData( '

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: '' + + '' + + '' + + '', + attributes: { + 1: { + styles: { + color: 'blue' + } + }, + 2: { + styles: { + background: 'pink' + } + } + } + } ); + + expect( editor.getData() ).to.equal( '

' ); + } ); + + it( 'should disallow attributes (classes)', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', classes: true } ); + dataFilter.disallowAttributes( { name: 'i', classes: [ 'bar' ] } ); + + editor.setData( '

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: '' + + '' + + '' + + '', + attributes: { + 1: { + classes: [ 'foo' ] + }, + 2: { + classes: [ 'abc' ] + } + } + } ); + + expect( editor.getData() ).to.equal( '

' ); + } ); + + it( 'should disallow element if all attributes are disallowed (classes)', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', classes: true } ); + dataFilter.disallowAttributes( { name: 'i', classes: [ 'bar' ] } ); + + editor.setData( '

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: '' + + '' + + '', + attributes: { + 1: { + classes: [ 'foo' ] + } + } + } ); + + expect( editor.getData() ).to.equal( '

' ); + } ); + + it( 'should not convert empty inline element without any attributes', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', classes: true } ); + + editor.setData( '

foo bar

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: '' + + 'foo bar' + + '', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( '

foo bar

' ); + } ); + + it( 'should apply attributes to correct editing element', () => { + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', classes: true } ); + + editor.setData( '

' ); + + const empty = editor.editing.view.document.getRoot() + .getChild( 0 ) //

+ .getChild( 0 ); // + + expect( Array.from( empty.getClassNames() ) ).to.deep.equal( [ 'foo', 'ck-widget' ] ); + } ); + + it( 'should not insert if not allowed by model schema', () => { + model.schema.addChildCheck( ( context, childDefinition ) => { + if ( context.endsWith( 'paragraph' ) && childDefinition.isObject ) { + return false; + } + } ); + dataFilter.allowEmptyElement( 'i' ); + dataFilter.allowElement( 'i' ); + dataFilter.allowAttributes( { name: 'i', classes: true } ); + + editor.setData( '

foo bar

' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true } ) ).to.deep.equal( { + data: 'foo bar', + attributes: {} + } ); + + expect( editor.getData() ).to.equal( '

foo  bar

' ); + } ); + } ); + describe( 'attributes modifications', () => { let root; @@ -3828,6 +4047,72 @@ describe( 'DataFilter', () => { } ); } ); + describe( 'loadAllowedEmptyElementsConfig', () => { + it( 'should allow empty element by name', async () => { + const editorElement = document.createElement( 'div' ); + + document.body.appendChild( editorElement ); + + const editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Paragraph, FontColorEditing, LinkEditing, GeneralHtmlSupport ], + htmlSupport: { + allow: [ { name: 'i', classes: true }, { name: 'b', classes: true } ], + allowEmpty: [ 'i' ] + } + } ); + + editor.setData( '

foobarbaz

' ); + + expect( getModelDataWithAttributes( editor.model, { withoutSelection: true } ) ).to.deep.equal( { + data: 'foobarbaz', + attributes: { + 1: { + classes: [ 'a' ] + } + } + } ); + expect( editor.getData() ).to.equal( '

foobarbaz

' ); + + editorElement.remove(); + await editor.destroy(); + } ); + + it( 'should allow multiple empty element by name', async () => { + const editorElement = document.createElement( 'div' ); + + document.body.appendChild( editorElement ); + + const editor = await ClassicTestEditor.create( editorElement, { + plugins: [ Paragraph, FontColorEditing, LinkEditing, GeneralHtmlSupport ], + htmlSupport: { + allow: [ { name: 'i', classes: true }, { name: 'b', classes: true } ], + allowEmpty: [ 'i', 'b' ] + } + } ); + + editor.setData( '

foobarbaz

' ); + + expect( getModelDataWithAttributes( editor.model, { withoutSelection: true } ) ).to.deep.equal( { + data: + '' + + 'foobarbaz' + + '', + attributes: { + 1: { + classes: [ 'a' ] + }, + 2: { + classes: [ 'b' ] + } + } + } ); + expect( editor.getData() ).to.equal( '

foobarbaz

' ); + + editorElement.remove(); + await editor.destroy(); + } ); + } ); + describe( 'loadAllowedConfig', () => { it( 'should allow match all elements by omitting pattern name', () => { dataSchema.registerBlockElement( { diff --git a/packages/ckeditor5-html-support/tests/integrations/customelement.js b/packages/ckeditor5-html-support/tests/integrations/customelement.js index 045c30c26d7..ef2b7b10e33 100644 --- a/packages/ckeditor5-html-support/tests/integrations/customelement.js +++ b/packages/ckeditor5-html-support/tests/integrations/customelement.js @@ -231,13 +231,13 @@ describe( 'CustomElementSupport', () => { data: 'foo ' + '' + 'bar', attributes: {} } ); - expect( editor.getData() ).to.equal( '

foo this is

some content

and more of it bar

' ); + expect( editor.getData() ).to.equal( '

foo this is

some content

and more of it bar

' ); } ); it( 'should not inject nbsp in the element content', () => { diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-all-features.html b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.html index 1816b66eeca..117f1db225f 100644 --- a/packages/ckeditor5-html-support/tests/manual/ghs-all-features.html +++ b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.html @@ -15,6 +15,12 @@ .text-italic { font-style: italic; } + i.inline-icon { + display: inline-block; + width: 1em; + height: 1em; + background: red; + }
@@ -135,6 +141,10 @@

Feature heading3

HTML snippet
 
+ +

empty inline at start

+

Text with empty inline inside

+

Text with empty inline at the end

diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-all-features.js b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.js index 0c09d3a417e..bb4423a4a7d 100644 --- a/packages/ckeditor5-html-support/tests/manual/ghs-all-features.js +++ b/packages/ckeditor5-html-support/tests/manual/ghs-all-features.js @@ -84,7 +84,8 @@ ClassicEditor attributes: true, classes: true } - ] + ], + allowEmpty: [ 'i' ] } } ) .then( editor => { diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.html b/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.html new file mode 100644 index 00000000000..18383b7b4e5 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.html @@ -0,0 +1,45 @@ + + + + +
+
+
+

empty inline at start.

+

Text with empty inline inside.

+

Text with empty inline at the end

+

An empty [ ] inline without any attributes (should be dropped).

+

An empty [ ] with attributes but no display-block.

+

Styled as display-block is [ ] here.

+
+
+ +
+

Source content:

+
+
+
+ diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.js b/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.js new file mode 100644 index 00000000000..bb4423a4a7d --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.js @@ -0,0 +1,96 @@ +/** + * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals console:false, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import Alignment from '@ckeditor/ckeditor5-alignment/src/alignment'; +import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; +import Code from '@ckeditor/ckeditor5-basic-styles/src/code'; +import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock'; +import EasyImage from '@ckeditor/ckeditor5-easy-image/src/easyimage'; +import FontBackgroundColor from '@ckeditor/ckeditor5-font/src/fontbackgroundcolor'; +import FontColor from '@ckeditor/ckeditor5-font/src/fontcolor'; +import FontFamily from '@ckeditor/ckeditor5-font/src/fontfamily'; +import FontSize from '@ckeditor/ckeditor5-font/src/fontsize'; +import Highlight from '@ckeditor/ckeditor5-highlight/src/highlight'; +import HorizontalLine from '@ckeditor/ckeditor5-horizontal-line/src/horizontalline'; +import HtmlEmbed from '@ckeditor/ckeditor5-html-embed/src/htmlembed'; +import ImageResize from '@ckeditor/ckeditor5-image/src/imageresize'; +import IndentBlock from '@ckeditor/ckeditor5-indent/src/indentblock'; +import LinkImage from '@ckeditor/ckeditor5-link/src/linkimage'; +import ListProperties from '@ckeditor/ckeditor5-list/src/listproperties'; +import PageBreak from '@ckeditor/ckeditor5-page-break/src/pagebreak'; +import Strikethrough from '@ckeditor/ckeditor5-basic-styles/src/strikethrough'; +import Subscript from '@ckeditor/ckeditor5-basic-styles/src/subscript'; +import Superscript from '@ckeditor/ckeditor5-basic-styles/src/superscript'; +import SourceEditing from '@ckeditor/ckeditor5-source-editing/src/sourceediting'; +import TableCellProperties from '@ckeditor/ckeditor5-table/src/tablecellproperties'; +import TableProperties from '@ckeditor/ckeditor5-table/src/tableproperties'; +import TableCaption from '@ckeditor/ckeditor5-table/src/tablecaption'; +import TableColumnResize from '@ckeditor/ckeditor5-table/src/tablecolumnresize'; +import TodoList from '@ckeditor/ckeditor5-list/src/todolist'; +import Underline from '@ckeditor/ckeditor5-basic-styles/src/underline'; +import CloudServices from '@ckeditor/ckeditor5-cloud-services/src/cloudservices'; +import ImageUpload from '@ckeditor/ckeditor5-image/src/imageupload'; +import RemoveFormat from '@ckeditor/ckeditor5-remove-format/src/removeformat'; + +import GeneralHtmlSupport from '../../src/generalhtmlsupport'; + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ + ArticlePluginSet, Underline, Strikethrough, Superscript, Subscript, Code, + FontColor, FontBackgroundColor, FontFamily, FontSize, Highlight, + CodeBlock, TodoList, ListProperties, TableProperties, TableCellProperties, TableCaption, + TableColumnResize, EasyImage, ImageResize, LinkImage, HtmlEmbed, + Alignment, IndentBlock, + PageBreak, HorizontalLine, + ImageUpload, CloudServices, + RemoveFormat, + SourceEditing, + GeneralHtmlSupport + ], + toolbar: [ + 'sourceEditing', + '|', + 'heading', + '|', + 'bold', 'italic', 'strikethrough', 'underline', 'code', 'subscript', 'superscript', 'link', + '|', + 'highlight', 'fontSize', 'fontFamily', 'fontColor', 'fontBackgroundColor', + '|', + 'bulletedList', 'numberedList', 'todoList', + '|', + 'blockQuote', 'uploadImage', 'insertTable', 'mediaEmbed', 'codeBlock', + '|', + 'htmlEmbed', + '|', + 'alignment', 'outdent', 'indent', + '|', + 'pageBreak', 'horizontalLine', + '|', + 'undo', 'redo', + '|', + 'RemoveFormat' + ], + htmlSupport: { + allow: [ + { + name: /^.*$/, + styles: true, + attributes: true, + classes: true + } + ], + allowEmpty: [ 'i' ] + } + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.md b/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.md new file mode 100644 index 00000000000..3bafe577396 --- /dev/null +++ b/packages/ckeditor5-html-support/tests/manual/ghs-empty-inline.md @@ -0,0 +1,3 @@ +## GHS empty inline elements + +Compare editor HTML with the raw HTML on the right side. This test is focused on empty inline elements. diff --git a/packages/ckeditor5-link/tests/linkcommand.js b/packages/ckeditor5-link/tests/linkcommand.js index 9b8c0f88b1b..0c6e1e97b5e 100644 --- a/packages/ckeditor5-link/tests/linkcommand.js +++ b/packages/ckeditor5-link/tests/linkcommand.js @@ -262,15 +262,12 @@ describe( 'LinkCommand', () => { expect( command.value ).to.be.equal( 'foo' ); } ); - // NOTE: The command value should most likely be "foo" but this requires a lot changes in refresh() - // because it relies on getSelectedElement()/getSelectedBlocks() and neither will return the inline widget - // in this case. - it( 'should not read the value from a selected linkable when a linked text follows it', () => { + it( 'should read the value from a selected linkable when a linked text follows it', () => { setData( model, '[<$text linkHref="bar">bar]' ); - expect( command.value ).to.be.undefined; + expect( command.value ).to.be.equal( 'foo' ); } ); it( 'should read the value from a selected text node and ignore a linkable', () => { diff --git a/packages/ckeditor5-source-editing/src/utils/formathtml.ts b/packages/ckeditor5-source-editing/src/utils/formathtml.ts index c0ff123d841..d1e440c8774 100644 --- a/packages/ckeditor5-source-editing/src/utils/formathtml.ts +++ b/packages/ckeditor5-source-editing/src/utils/formathtml.ts @@ -25,12 +25,12 @@ export function formatHtml( input: string ): string { // a full list of HTML block-level elements. // A void element is an element that cannot have any child - https://html.spec.whatwg.org/multipage/syntax.html#void-elements. // Note that
 element is not listed on this list to avoid breaking whitespace formatting.
+	// Note that 
element is not listed and handled separately so no additional white spaces are injected. const elementsToFormat: Array = [ { name: 'address', isVoid: false }, { name: 'article', isVoid: false }, { name: 'aside', isVoid: false }, { name: 'blockquote', isVoid: false }, - { name: 'br', isVoid: true }, { name: 'details', isVoid: false }, { name: 'dialog', isVoid: false }, { name: 'dd', isVoid: false }, @@ -75,6 +75,8 @@ export function formatHtml( input: string ): string { // Add new line before and after `` and ``. // It may separate individual elements with two new lines, but this will be fixed below. .replace( new RegExp( ``, 'g' ), '\n$&\n' ) + // Keep `
`s at the end of line to avoid adding additional whitespaces before `
`. + .replace( /]*>/g, '$&\n' ) // Divide input string into lines, which start with either an opening tag, a closing tag, or just a text. .split( '\n' ); diff --git a/packages/ckeditor5-source-editing/tests/utils/formathtml.js b/packages/ckeditor5-source-editing/tests/utils/formathtml.js index 87fb530feaa..57b551f59e0 100644 --- a/packages/ckeditor5-source-editing/tests/utils/formathtml.js +++ b/packages/ckeditor5-source-editing/tests/utils/formathtml.js @@ -153,8 +153,7 @@ describe( 'SourceEditing utils', () => { ' \n' + '
\n' + '
\n' + - ' john@example.com\n' + - '
\n' + + ' john@example.com
\n' + ' (310) 555-1234\n' + '
\n' + ''; @@ -235,12 +234,10 @@ describe( 'SourceEditing utils', () => { // This is not pretty but at least it does not crash. const sourceFormatted = - '

\n' + - ' \n' + - '

'; + '

\n' + + ' \n' + + '

'; expect( formatHtml( source ) ).to.equal( sourceFormatted ); } ); diff --git a/packages/ckeditor5-typing/src/twostepcaretmovement.ts b/packages/ckeditor5-typing/src/twostepcaretmovement.ts index 58af6141d2d..1c4becd5990 100644 --- a/packages/ckeditor5-typing/src/twostepcaretmovement.ts +++ b/packages/ckeditor5-typing/src/twostepcaretmovement.ts @@ -460,7 +460,19 @@ function setSelectionAttributesFromTheNodeBefore( model: Model, attributes: Set< const nodeBefore = position.nodeBefore; model.change( writer => { if ( nodeBefore ) { - writer.setSelectionAttribute( nodeBefore.getAttributes() ); + const attributes: Array<[string, unknown]> = []; + const isInlineObject = model.schema.isObject( nodeBefore ) && model.schema.isInline( nodeBefore ); + + for ( const [ key, value ] of nodeBefore.getAttributes() ) { + if ( + model.schema.checkAttribute( '$text', key ) && + ( !isInlineObject || model.schema.getAttributeProperties( key ).copyFromObject !== false ) + ) { + attributes.push( [ key, value ] ); + } + } + + writer.setSelectionAttribute( attributes ); } else { writer.removeSelectionAttribute( attributes ); } diff --git a/packages/ckeditor5-typing/tests/twostepcaretmovement.js b/packages/ckeditor5-typing/tests/twostepcaretmovement.js index d10bb4f09b3..355760628e6 100644 --- a/packages/ckeditor5-typing/tests/twostepcaretmovement.js +++ b/packages/ckeditor5-typing/tests/twostepcaretmovement.js @@ -40,12 +40,18 @@ describe( 'TwoStepCaretMovement()', () => { allowAttributes: [ 'a', 'b', 'c' ], allowIn: '$root' } ); + editor.model.schema.register( 'inlineObject', { + inheritAllFrom: '$inlineObject', + allowAttributes: [ 'src' ] + } ); model.schema.register( 'paragraph', { inheritAllFrom: '$block' } ); editor.conversion.for( 'upcast' ).elementToAttribute( { view: 'a', model: 'a' } ); editor.conversion.for( 'upcast' ).elementToAttribute( { view: 'b', model: 'b' } ); editor.conversion.for( 'upcast' ).elementToAttribute( { view: 'c', model: 'c' } ); editor.conversion.elementToElement( { model: 'paragraph', view: 'p' } ); + editor.conversion.elementToElement( { model: 'inlineObject', view: 'inlineObject' } ); + editor.conversion.attributeToAttribute( { model: 'src', view: 'src' } ); plugin.registerAttribute( 'a' ); } ); @@ -216,6 +222,31 @@ describe( 'TwoStepCaretMovement()', () => { { selectionAttributes: [ 'b' ], isGravityOverridden: false, preventDefault: 1, evtStop: 1 } ] ); } ); + + it( 'should copy attributes from an inline object if are allowed on text', () => { + setData( model, 'fo[]o' ); + + testTwoStepCaretMovement( [ + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 2 ] }, + '→', + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 3 ] }, + '→', + { selectionAttributes: [ 'a', 'b' ], isGravityOverridden: true, preventDefault: 1, evtStop: 1, caretPosition: [ 0, 3 ] } + ] ); + } ); + + it( 'should copy attributes from an inline object if are allowed on text and not disabled by copyFromObject', () => { + model.schema.setAttributeProperties( 'c', { copyFromObject: false } ); + setData( model, 'fo[]o' ); + + testTwoStepCaretMovement( [ + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 2 ] }, + '→', + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 3 ] }, + '→', + { selectionAttributes: [ 'a', 'b' ], isGravityOverridden: true, preventDefault: 1, evtStop: 1, caretPosition: [ 0, 3 ] } + ] ); + } ); } ); describe( 'moving left', () => { @@ -412,6 +443,31 @@ describe( 'TwoStepCaretMovement()', () => { { selectionAttributes: [ 'b' ], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: [ 0, 3 ] } ] ); } ); + + it( 'should copy attributes from an inline object if are allowed on text', () => { + setData( model, 'f[]oo' ); + + testTwoStepCaretMovement( [ + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 2 ] }, + '←', + { selectionAttributes: [], isGravityOverridden: true, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 1 ] }, + '←', + { selectionAttributes: [ 'a', 'b' ], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: [ 0, 1 ] } + ] ); + } ); + + it( 'should copy attributes from an inline object if are allowed on text and not disabled by copyFromObject', () => { + model.schema.setAttributeProperties( 'c', { copyFromObject: false } ); + setData( model, 'f[]oo' ); + + testTwoStepCaretMovement( [ + { selectionAttributes: [], isGravityOverridden: false, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 2 ] }, + '←', + { selectionAttributes: [], isGravityOverridden: true, preventDefault: 0, evtStop: 0, caretPosition: [ 0, 1 ] }, + '←', + { selectionAttributes: [ 'a', 'b' ], isGravityOverridden: false, preventDefault: 1, evtStop: 1, caretPosition: [ 0, 1 ] } + ] ); + } ); } ); describe( 'moving and typing around the attribute', () => { @@ -983,4 +1039,3 @@ describe( 'TwoStepCaretMovement()', () => { } } } ); -