From 8039a8d4ce86eadcab13ec9d5ffb74d4f700e09a Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 28 Feb 2020 13:00:16 +0100 Subject: [PATCH] Added a new method (#isAttached()) to both (view and model) Nodes classes. --- src/conversion/downcasthelpers.js | 2 +- src/model/node.js | 9 ++++++ src/view/element.js | 4 --- src/view/node.js | 9 ++++++ tests/model/node.js | 51 +++++++++++++++++++++++++++++++ tests/view/node.js | 43 +++++++++++++++++++------- tests/view/position.js | 1 - 7 files changed, 102 insertions(+), 17 deletions(-) diff --git a/src/conversion/downcasthelpers.js b/src/conversion/downcasthelpers.js index d80e1a4ee..9ed599f93 100644 --- a/src/conversion/downcasthelpers.js +++ b/src/conversion/downcasthelpers.js @@ -544,7 +544,7 @@ export function clearAttributes() { // Not collapsed selection should not have artifacts. if ( range.isCollapsed ) { // Position might be in the node removed by the view writer. - if ( range.end.parent.parent ) { + if ( range.end.parent.isAttached() ) { conversionApi.writer.mergeAttributes( range.start ); } } diff --git a/src/model/node.js b/src/model/node.js index 5c5800139..24485f5ee 100644 --- a/src/model/node.js +++ b/src/model/node.js @@ -188,6 +188,15 @@ export default class Node { return root; } + /** + * Returns true if a node is in a tree rooted in an element of the root type. + * + * @returns {Boolean} + */ + isAttached() { + return this.root.is( 'rootElement' ); + } + /** * Gets path to the node. The path is an array containing starting offsets of consecutive ancestors of this node, * beginning from {@link module:engine/model/node~Node#root root}, down to this node's starting offset. The path can be used to diff --git a/src/view/element.js b/src/view/element.js index 7443b1ac6..a15dbe7c8 100644 --- a/src/view/element.js +++ b/src/view/element.js @@ -667,10 +667,6 @@ export default class Element extends Node { if ( key == 'class' ) { parseClasses( this._classes, value ); } else if ( key == 'style' ) { - // if (!this._styles ) { - // debugger; - // } - this._styles.setTo( value ); } else { this._attrs.set( key, value ); diff --git a/src/view/node.js b/src/view/node.js index c4b7a4300..f33f5461a 100644 --- a/src/view/node.js +++ b/src/view/node.js @@ -117,6 +117,15 @@ export default class Node { return root; } + /** + * Returns true if a node is in a tree rooted in an element of the root type. + * + * @returns {Boolean} + */ + isAttached() { + return this.root.is( 'rootElement' ); + } + /** * Gets a path to the node. The path is an array containing indices of consecutive ancestors of this node, * beginning from {@link module:engine/view/node~Node#root root}, down to this node's index. diff --git a/tests/model/node.js b/tests/model/node.js index f684fdfc9..965df900b 100644 --- a/tests/model/node.js +++ b/tests/model/node.js @@ -8,8 +8,10 @@ import DocumentFragment from '../../src/model/documentfragment'; import Node from '../../src/model/node'; import Element from '../../src/model/element'; import Text from '../../src/model/text'; +import RootElement from '../../src/model/rootelement'; import count from '@ckeditor/ckeditor5-utils/src/count'; import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; +import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; describe( 'Node', () => { let doc, root, node, @@ -386,6 +388,55 @@ describe( 'Node', () => { } ); } ); + describe( 'isAttached()', () => { + it( 'returns false for a fresh node', () => { + const char = new Text( 'x' ); + const el = new Element( 'one' ); + + expect( char.isAttached() ).to.equal( false ); + expect( el.isAttached() ).to.equal( false ); + } ); + + it( 'returns true for the root element', () => { + const model = new Model(); + const root = new RootElement( model.document, 'root' ); + + expect( root.isAttached() ).to.equal( true ); + } ); + + it( 'returns false for a node attached to a document fragment', () => { + const foo = new Text( 'foo' ); + new DocumentFragment( [ foo ] ); // eslint-disable-line no-new + + expect( foo.isAttached() ).to.equal( false ); + } ); + + it( 'returns true for a node moved to graveyard', () => { + return ModelTestEditor.create() + .then( editor => { + const model = editor.model; + const root = model.document.getRoot(); + + // Allow "paragraph" element to be added as a child in block elements. + model.schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + + const node = model.change( writer => writer.createElement( 'paragraph' ) ); + + expect( node.isAttached() ).to.equal( false ); + + model.change( writer => writer.append( node, root ) ); + + expect( node.isAttached() ).to.equal( true ); + + model.change( writer => writer.remove( node ) ); + + expect( node.isAttached() ).to.equal( true ); + + return editor.destroy(); + } ); + } ); + } ); + describe( 'attributes interface', () => { const node = new Node( { foo: 'bar' } ); diff --git a/tests/view/node.js b/tests/view/node.js index 53d81b16b..465abffed 100644 --- a/tests/view/node.js +++ b/tests/view/node.js @@ -9,7 +9,6 @@ import Node from '../../src/view/node'; import DocumentFragment from '../../src/view/documentfragment'; import RootEditableElement from '../../src/view/rooteditableelement'; -import createDocumentMock from '../../tests/view/_utils/createdocumentmock'; import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; import Document from '../../src/view/document'; import { StylesProcessor } from '../../src/view/stylesmap'; @@ -180,9 +179,9 @@ describe( 'Node', () => { } ); it( 'should return proper element for nodes in different branches and on different levels', () => { - const foo = new Text( 'foo' ); - const bar = new Text( 'bar' ); - const bom = new Text( 'bom' ); + const foo = new Text( document, 'foo' ); + const bar = new Text( document, 'bar' ); + const bom = new Text( document, 'bom' ); const d = new Element( document, 'd', null, [ bar ] ); const c = new Element( document, 'c', null, [ foo, d ] ); const b = new Element( document, 'b', null, [ c ] ); @@ -207,8 +206,8 @@ describe( 'Node', () => { } ); it( 'should return document fragment', () => { - const foo = new Text( 'foo' ); - const bar = new Text( 'bar' ); + const foo = new Text( document, 'foo' ); + const bar = new Text( document, 'bar' ); const df = new DocumentFragment( document, [ foo, bar ] ); expect( foo.getCommonAncestor( bar ) ).to.equal( df ); @@ -232,7 +231,7 @@ describe( 'Node', () => { } ); it( 'should throw an error if parent does not contain element', () => { - const f = new Text( 'f' ); + const f = new Text( document, 'f' ); const bar = new Element( document, 'bar', [], [] ); f.parent = bar; @@ -266,7 +265,6 @@ describe( 'Node', () => { it( 'should return root element', () => { const parent = new RootEditableElement( document, 'div' ); - parent._document = createDocumentMock(); const child = new Element( document, 'p' ); child.parent = parent; @@ -352,9 +350,32 @@ describe( 'Node', () => { } ); } ); + describe( 'isAttached()', () => { + it( 'returns false for a fresh node', () => { + const char = new Text( document, 'x' ); + const el = new Element( document, 'one' ); + + expect( char.isAttached() ).to.equal( false ); + expect( el.isAttached() ).to.equal( false ); + } ); + + it( 'returns true for the root element', () => { + const root = new RootEditableElement( document, 'div' ); + + expect( root.isAttached() ).to.equal( true ); + } ); + + it( 'returns false for a node attached to a document fragment', () => { + const foo = new Text( document, 'foo' ); + new DocumentFragment( document, [ foo ] ); // eslint-disable-line no-new + + expect( foo.isAttached() ).to.equal( false ); + } ); + } ); + describe( '_remove()', () => { it( 'should remove node from its parent', () => { - const char = new Text( 'a' ); + const char = new Text( document, 'a' ); const parent = new Element( document, 'p', null, [ char ] ); char._remove(); @@ -362,7 +383,7 @@ describe( 'Node', () => { } ); it( 'uses parent._removeChildren method', () => { - const char = new Text( 'a' ); + const char = new Text( document, 'a' ); const parent = new Element( document, 'p', null, [ char ] ); const _removeChildrenSpy = sinon.spy( parent, '_removeChildren' ); const index = char.index; @@ -399,7 +420,7 @@ describe( 'Node', () => { } ); beforeEach( () => { - text = new Text( 'foo' ); + text = new Text( document, 'foo' ); img = new Element( document, 'img', { 'src': 'img.png' } ); root = new Element( document, 'p', { renderer: { markToSync: rootChangeSpy } } ); diff --git a/tests/view/position.js b/tests/view/position.js index 9e5807fdd..1bd715959 100644 --- a/tests/view/position.js +++ b/tests/view/position.js @@ -546,7 +546,6 @@ describe( 'Position', () => { it( 'should return EditableElement when position is placed inside', () => { const p = new Element( document, 'p' ); const editable = new EditableElement( document, 'div', null, p ); - editable._document = document; const position = new Position( p, 0 ); expect( position.editableElement ).to.equal( editable );