diff --git a/src/controller/deletecontent.js b/src/controller/deletecontent.js index 7a9a91e32..8a9efd8bb 100644 --- a/src/controller/deletecontent.js +++ b/src/controller/deletecontent.js @@ -41,9 +41,11 @@ export default function deleteContent( selection, batch, options = {} ) { return; } + const schema = batch.document.schema; + // 1. Replace the entire content with paragraph. // See: https://github.com/ckeditor/ckeditor5-engine/issues/1012#issuecomment-315017594. - if ( !options.doNotResetEntireContent && shouldEntireContentBeReplacedWithParagraph( batch.document.schema, selection ) ) { + if ( !options.doNotResetEntireContent && shouldEntireContentBeReplacedWithParagraph( schema, selection ) ) { replaceEntireContentWithParagraph( batch, selection ); return; @@ -74,14 +76,14 @@ export default function deleteContent( selection, batch, options = {} ) { // // e.g. bold is disallowed for

//

Fo{o

b}ar

->

Fo{}ar

->

Fo{}ar

. - removeDisallowedAttributes( startPos.parent.getChildren(), startPos, batch ); + schema.removeDisallowedAttributes( startPos.parent.getChildren(), startPos, batch ); } selection.setCollapsedAt( startPos ); // 4. Autoparagraphing. // Check if a text is allowed in the new container. If not, try to create a new paragraph (if it's allowed here). - if ( shouldAutoparagraph( batch.document, startPos ) ) { + if ( shouldAutoparagraph( schema, startPos ) ) { insertParagraph( batch, startPos, selection ); } @@ -158,9 +160,9 @@ function mergeBranches( batch, startPos, endPos ) { mergeBranches( batch, startPos, endPos ); } -function shouldAutoparagraph( doc, position ) { - const isTextAllowed = doc.schema.check( { name: '$text', inside: position } ); - const isParagraphAllowed = doc.schema.check( { name: 'paragraph', inside: position } ); +function shouldAutoparagraph( schema, position ) { + const isTextAllowed = schema.check( { name: '$text', inside: position } ); + const isParagraphAllowed = schema.check( { name: 'paragraph', inside: position } ); return !isTextAllowed && isParagraphAllowed; } @@ -217,39 +219,3 @@ function shouldEntireContentBeReplacedWithParagraph( schema, selection ) { return schema.check( { name: 'paragraph', inside: limitElement.name } ); } - -// Gets a name under which we should check this node in the schema. -// -// @param {module:engine/model/node~Node} node The node. -// @returns {String} node name. -function getNodeSchemaName( node ) { - return node.is( 'text' ) ? '$text' : node.name; -} - -// Creates AttributeDeltas that removes attributes that are disallowed by schema on given node and its children. -// -// @param {Array} nodes Nodes that will be filtered. -// @param {module:engine/model/schema~SchemaPath} inside Path inside which schema will be checked. -// @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added. -function removeDisallowedAttributes( nodes, inside, batch ) { - const schema = batch.document.schema; - - for ( const node of nodes ) { - const name = getNodeSchemaName( node ); - - // When node with attributes is not allowed in current position. - if ( !schema.check( { name, inside, attributes: Array.from( node.getAttributeKeys() ) } ) ) { - // Let's remove attributes one by one. - // This should be improved to check all combination of attributes. - for ( const attribute of node.getAttributeKeys() ) { - if ( !schema.check( { name, inside, attributes: attribute } ) ) { - batch.removeAttribute( node, attribute ); - } - } - } - - if ( node.is( 'element' ) ) { - removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), batch ); - } - } -} diff --git a/src/controller/insertcontent.js b/src/controller/insertcontent.js index c8e24150e..52a2e9902 100644 --- a/src/controller/insertcontent.js +++ b/src/controller/insertcontent.js @@ -229,7 +229,7 @@ class Insertion { // If the node is a text and bare text is allowed in current position it means that the node // contains disallowed attributes and we have to remove them. else if ( this.schema.check( { name: '$text', inside: this.position } ) ) { - removeDisallowedAttributes( [ node ], this.position, this.schema ); + this.schema.removeDisallowedAttributes( [ node ], this.position ); this._handleNode( node, context ); } // If text is not allowed, try autoparagraphing. @@ -291,7 +291,7 @@ class Insertion { // We need to check and strip disallowed attributes in all nested nodes because after merge // some attributes could end up in a path where are disallowed. const parent = position.nodeBefore; - removeDisallowedAttributes( parent.getChildren(), Position.createAt( parent ), this.schema, this.batch ); + this.schema.removeDisallowedAttributes( parent.getChildren(), Position.createAt( parent ), this.batch ); this.position = Position.createFromPosition( position ); position.detach(); @@ -318,7 +318,7 @@ class Insertion { // We need to check and strip disallowed attributes in all nested nodes because after merge // some attributes could end up in a place where are disallowed. - removeDisallowedAttributes( position.parent.getChildren(), position, this.schema, this.batch ); + this.schema.removeDisallowedAttributes( position.parent.getChildren(), position, this.batch ); this.position = Position.createFromPosition( position ); position.detach(); @@ -330,7 +330,7 @@ class Insertion { // When there was no merge we need to check and strip disallowed attributes in all nested nodes of // just inserted node because some attributes could end up in a place where are disallowed. if ( !mergeLeft && !mergeRight ) { - removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), this.schema, this.batch ); + this.schema.removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), this.batch ); } } @@ -350,7 +350,7 @@ class Insertion { // When node is a text and is disallowed by schema it means that contains disallowed attributes // and we need to remove them. if ( node.is( 'text' ) && !this._checkIsAllowed( node, [ paragraph ] ) ) { - removeDisallowedAttributes( [ node ], [ paragraph ], this.schema ); + this.schema.removeDisallowedAttributes( [ node ], [ paragraph ] ); } if ( this._checkIsAllowed( node, [ paragraph ] ) ) { @@ -453,36 +453,3 @@ class Insertion { function getNodeSchemaName( node ) { return node.is( 'text' ) ? '$text' : node.name; } - -// Removes disallowed by schema attributes from given nodes. When batch parameter is provided then -// attributes will be removed by creating AttributeDeltas otherwise attributes will be removed -// directly from provided nodes. -// -// @param {Array} nodes Nodes that will be filtered. -// @param {module:engine/model/schema~SchemaPath} inside Path inside which schema will be checked. -// @param {module:engine/model/schema~Schema} schema Schema instance uses for element validation. -// @param {module:engine/model/batch~Batch} [batch] Batch to which the deltas will be added. -function removeDisallowedAttributes( nodes, inside, schema, batch ) { - for ( const node of nodes ) { - const name = getNodeSchemaName( node ); - - // When node with attributes is not allowed in current position. - if ( !schema.check( { name, inside, attributes: Array.from( node.getAttributeKeys() ) } ) ) { - // Let's remove attributes one by one. - // This should be improved to check all combination of attributes. - for ( const attribute of node.getAttributeKeys() ) { - if ( !schema.check( { name, inside, attributes: attribute } ) ) { - if ( batch ) { - batch.removeAttribute( node, attribute ); - } else { - node.removeAttribute( attribute ); - } - } - } - } - - if ( node.is( 'element' ) ) { - removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), schema, batch ); - } - } -} diff --git a/src/model/schema.js b/src/model/schema.js index f18eae5e2..5f927c3a9 100644 --- a/src/model/schema.js +++ b/src/model/schema.js @@ -9,11 +9,11 @@ import Position from './position'; import Element from './element'; +import Range from './range'; import clone from '@ckeditor/ckeditor5-utils/src/lib/lodash/clone'; import isArray from '@ckeditor/ckeditor5-utils/src/lib/lodash/isArray'; import isString from '@ckeditor/ckeditor5-utils/src/lib/lodash/isString'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; -import Range from './range'; /** * Schema is a definition of the structure of the document. It allows to define which tree model items (element, text, etc.) @@ -415,6 +415,43 @@ export default class Schema { return element; } + /** + * Removes disallowed by {@link module:engine/model/schema~Schema schema} attributes from given nodes. + * When {@link module:engine/model/batch~Batch batch} parameter is provided then attributes will be removed + * using that batch, by creating {@link module:engine/model/delta/attributedelta~AttributeDelta attribute deltas}. + * Otherwise, attributes will be removed directly from provided nodes using {@link module:engine/model/node~Node node} API. + * + * @param {Iterable.} nodes Nodes that will be filtered. + * @param {module:engine/model/schema~SchemaPath} inside Path inside which schema will be checked. + * @param {module:engine/model/batch~Batch} [batch] Batch to which the deltas will be added. + */ + removeDisallowedAttributes( nodes, inside, batch ) { + for ( const node of nodes ) { + const name = node.is( 'text' ) ? '$text' : node.name; + const attributes = Array.from( node.getAttributeKeys() ); + const queryPath = Schema._normalizeQueryPath( inside ); + + // When node with attributes is not allowed in current position. + if ( !this.check( { name, attributes, inside: queryPath } ) ) { + // Let's remove attributes one by one. + // TODO: this should be improved to check all combination of attributes. + for ( const attribute of node.getAttributeKeys() ) { + if ( !this.check( { name, attributes: attribute, inside: queryPath } ) ) { + if ( batch ) { + batch.removeAttribute( node, attribute ); + } else { + node.removeAttribute( attribute ); + } + } + } + } + + if ( node.is( 'element' ) ) { + this.removeDisallowedAttributes( node.getChildren(), queryPath.concat( node.name ), batch ); + } + } + } + /** * Returns {@link module:engine/model/schema~SchemaItem schema item} that was registered in the schema under given name. * If item has not been found, throws error. diff --git a/tests/model/schema/schema.js b/tests/model/schema/schema.js index 3a3a26a0d..656c77a2f 100644 --- a/tests/model/schema/schema.js +++ b/tests/model/schema/schema.js @@ -6,12 +6,15 @@ import { default as Schema, SchemaItem } from '../../../src/model/schema'; import Document from '../../../src/model/document'; import Element from '../../../src/model/element'; +import Text from '../../../src/model/text'; +import DocumentFragment from '../../../src/model/documentfragment'; import Position from '../../../src/model/position'; import Range from '../../../src/model/range'; import Selection from '../../../src/model/selection'; +import AttributeDelta from '../../../src/model/delta/attributedelta'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; -import { setData, stringify } from '../../../src/dev-utils/model'; +import { setData, getData, stringify } from '../../../src/dev-utils/model'; testUtils.createSinonSandbox(); @@ -752,4 +755,164 @@ describe( 'Schema', () => { expect( schema.getLimitElement( doc.selection ) ).to.equal( root ); } ); } ); + + describe( 'removeDisallowedAttributes()', () => { + let doc, root; + + beforeEach( () => { + doc = new Document(); + root = doc.createRoot(); + schema = doc.schema; + + schema.registerItem( 'paragraph', '$block' ); + schema.registerItem( 'div', '$block' ); + schema.registerItem( 'image' ); + schema.objects.add( 'image' ); + schema.allow( { name: '$block', inside: 'div' } ); + } ); + + describe( 'filtering attributes from nodes', () => { + let text, image; + + beforeEach( () => { + schema.allow( { name: '$text', attributes: [ 'a' ], inside: '$root' } ); + schema.allow( { name: 'image', attributes: [ 'b' ], inside: '$root' } ); + + text = new Text( 'foo', { a: 1, b: 1 } ); + image = new Element( 'image', { a: 1, b: 1 } ); + } ); + + it( 'should filter out disallowed attributes from given nodes', () => { + schema.removeDisallowedAttributes( [ text, image ], '$root' ); + + expect( Array.from( text.getAttributeKeys() ) ).to.deep.equal( [ 'a' ] ); + expect( Array.from( image.getAttributeKeys() ) ).to.deep.equal( [ 'b' ] ); + } ); + + it( 'should filter out disallowed attributes from given nodes (batch)', () => { + const root = doc.getRoot(); + const batch = doc.batch(); + + root.appendChildren( [ text, image ] ); + + schema.removeDisallowedAttributes( [ text, image ], '$root', batch ); + + expect( Array.from( text.getAttributeKeys() ) ).to.deep.equal( [ 'a' ] ); + expect( Array.from( image.getAttributeKeys() ) ).to.deep.equal( [ 'b' ] ); + + expect( batch.deltas ).to.length( 2 ); + expect( batch.deltas[ 0 ] ).to.instanceof( AttributeDelta ); + expect( batch.deltas[ 1 ] ).to.instanceof( AttributeDelta ); + } ); + } ); + + describe( 'filtering attributes from child nodes', () => { + let div; + + beforeEach( () => { + schema.allow( { name: '$text', attributes: [ 'a' ], inside: 'div' } ); + schema.allow( { name: '$text', attributes: [ 'b' ], inside: 'div paragraph' } ); + schema.allow( { name: 'image', attributes: [ 'a' ], inside: 'div' } ); + schema.allow( { name: 'image', attributes: [ 'b' ], inside: 'div paragraph' } ); + + const foo = new Text( 'foo', { a: 1, b: 1 } ); + const bar = new Text( 'bar', { a: 1, b: 1 } ); + const imageInDiv = new Element( 'image', { a: 1, b: 1 } ); + const imageInParagraph = new Element( 'image', { a: 1, b: 1 } ); + const paragraph = new Element( 'paragraph', [], [ foo, imageInParagraph ] ); + + div = new Element( 'div', [], [ paragraph, bar, imageInDiv ] ); + } ); + + it( 'should filter out disallowed attributes from child nodes', () => { + schema.removeDisallowedAttributes( [ div ], '$root' ); + + expect( stringify( div ) ) + .to.equal( + '
' + + '' + + '<$text b="1">foo' + + '' + + '' + + '<$text a="1">bar' + + '' + + '
' + ); + } ); + + it( 'should filter out disallowed attributes from child nodes (batch)', () => { + const root = doc.getRoot(); + const batch = doc.batch(); + + root.appendChildren( [ div ] ); + + schema.removeDisallowedAttributes( [ div ], '$root', batch ); + + expect( batch.deltas ).to.length( 4 ); + expect( batch.deltas[ 0 ] ).to.instanceof( AttributeDelta ); + expect( batch.deltas[ 1 ] ).to.instanceof( AttributeDelta ); + expect( batch.deltas[ 2 ] ).to.instanceof( AttributeDelta ); + expect( batch.deltas[ 3 ] ).to.instanceof( AttributeDelta ); + + expect( getData( doc, { withoutSelection: true } ) ) + .to.equal( + '
' + + '' + + '<$text b="1">foo' + + '' + + '' + + '<$text a="1">bar' + + '' + + '
' + ); + } ); + } ); + + describe( 'allowed parameters', () => { + let frag; + + beforeEach( () => { + schema.allow( { name: '$text', attributes: [ 'a' ], inside: '$root' } ); + schema.allow( { name: '$text', attributes: [ 'b' ], inside: 'paragraph' } ); + + frag = new DocumentFragment( [ + new Text( 'foo', { a: 1 } ), + new Element( 'paragraph', [], [ new Text( 'bar', { a: 1, b: 1 } ) ] ), + new Text( 'biz', { b: 1 } ) + ] ); + } ); + + it( 'should accept iterable as nodes', () => { + schema.removeDisallowedAttributes( frag.getChildren(), '$root' ); + + expect( stringify( frag ) ) + .to.equal( '<$text a="1">foo<$text b="1">barbiz' ); + } ); + + it( 'should accept Position as inside', () => { + schema.removeDisallowedAttributes( frag.getChildren(), Position.createAt( root ) ); + + expect( stringify( frag ) ) + .to.equal( '<$text a="1">foo<$text b="1">barbiz' ); + } ); + + it( 'should accept Node as inside', () => { + schema.removeDisallowedAttributes( frag.getChildren(), [ root ] ); + + expect( stringify( frag ) ) + .to.equal( '<$text a="1">foo<$text b="1">barbiz' ); + } ); + } ); + + it( 'should not filter out allowed combination of attributes', () => { + schema.allow( { name: 'image', attributes: [ 'a', 'b' ] } ); + schema.requireAttributes( 'image', [ 'a', 'b' ] ); + + const image = new Element( 'image', { a: 1, b: 1 } ); + + schema.removeDisallowedAttributes( [ image ], '$root' ); + + expect( Array.from( image.getAttributeKeys() ) ).to.deep.equal( [ 'a', 'b' ] ); + } ); + } ); } );