From a7765d27c446364931784e1aa4cf71f72f689b0f Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Tue, 25 Jul 2017 10:14:27 +0200 Subject: [PATCH 1/4] "model.Element#getNodeByPath()" works with offsets instead of indexes. --- src/model/element.js | 2 +- tests/model/element.js | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/model/element.js b/src/model/element.js index 44131a87e..5892e6000 100644 --- a/src/model/element.js +++ b/src/model/element.js @@ -241,7 +241,7 @@ export default class Element extends Node { let node = this; // eslint-disable-line consistent-this for ( const index of relativePath ) { - node = node.getChild( index ); + node = node.getChild( node.offsetToIndex( index ) ); } return node; diff --git a/tests/model/element.js b/tests/model/element.js index c331dde37..cb6ae2604 100644 --- a/tests/model/element.js +++ b/tests/model/element.js @@ -176,15 +176,48 @@ describe( 'Element', () => { } ); it( 'should return a descendant of this node', () => { + const foo = new Text( 'foo' ); const image = new Element( 'image' ); const element = new Element( 'elem', [], [ new Element( 'elem', [], [ - new Text( 'foo' ), + foo, image ] ) ] ); - expect( element.getNodeByPath( [ 0, 1 ] ) ).to.equal( image ); + expect( element.getNodeByPath( [ 0, 0 ] ) ).to.equal( foo ); + expect( element.getNodeByPath( [ 0, 1 ] ) ).to.equal( foo ); + expect( element.getNodeByPath( [ 0, 2 ] ) ).to.equal( foo ); + expect( element.getNodeByPath( [ 0, 3 ] ) ).to.equal( image ); + } ); + + it( 'works fine with offsets', () => { + const bar = new Text( 'bar' ); + const foo = new Text( 'foo' ); + const bom = new Text( 'bom' ); + const bold = new Element( 'b', [], [ + bar + ] ); + const paragraph = new Element( 'paragraph', [], [ + foo, + bold, + bom + ] ); + + // foobarbom + + expect( paragraph.getNodeByPath( [ 0 ] ) ).to.equal( foo ); + expect( paragraph.getNodeByPath( [ 1 ] ) ).to.equal( foo ); + expect( paragraph.getNodeByPath( [ 2 ] ) ).to.equal( foo ); + expect( paragraph.getNodeByPath( [ 3 ] ) ).to.equal( bold ); + expect( paragraph.getNodeByPath( [ 3, 0 ] ) ).to.equal( bar ); + expect( paragraph.getNodeByPath( [ 3, 1 ] ) ).to.equal( bar ); + expect( paragraph.getNodeByPath( [ 3, 2 ] ) ).to.equal( bar ); + expect( paragraph.getNodeByPath( [ 3, 3 ] ) ).to.equal( null ); + expect( paragraph.getNodeByPath( [ 4 ] ) ).to.equal( bom ); + expect( paragraph.getNodeByPath( [ 5 ] ) ).to.equal( bom ); + expect( paragraph.getNodeByPath( [ 6 ] ) ).to.equal( bom ); + expect( paragraph.getNodeByPath( [ 7 ] ) ).to.equal( null ); } ); } ); From 8bc50cba840247aea7770e3fbe81bb30d5acc4fa Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Tue, 25 Jul 2017 10:19:02 +0200 Subject: [PATCH 2/4] "model.DocumentFragment#getNodeByPath()" works with offsets instead of indexes. --- src/model/documentfragment.js | 2 +- tests/model/documentfragment.js | 38 +++++++++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/model/documentfragment.js b/src/model/documentfragment.js index 08056943e..626c4e097 100644 --- a/src/model/documentfragment.js +++ b/src/model/documentfragment.js @@ -186,7 +186,7 @@ export default class DocumentFragment { let node = this; // eslint-disable-line consistent-this for ( const index of relativePath ) { - node = node.getChild( index ); + node = node.getChild( node.offsetToIndex( index ) ); } return node; diff --git a/tests/model/documentfragment.js b/tests/model/documentfragment.js index 2d12626dd..707815f20 100644 --- a/tests/model/documentfragment.js +++ b/tests/model/documentfragment.js @@ -309,16 +309,50 @@ describe( 'DocumentFragment', () => { } ); it( 'should return a descendant of this node', () => { + const foo = new Text( 'foo' ); const image = new Element( 'image' ); const element = new Element( 'elem', [], [ new Element( 'elem', [], [ - new Text( 'foo' ), + foo, image ] ) ] ); const frag = new DocumentFragment( element ); - expect( frag.getNodeByPath( [ 0, 0, 1 ] ) ).to.equal( image ); + expect( frag.getNodeByPath( [ 0, 0, 0 ] ) ).to.equal( foo ); + expect( frag.getNodeByPath( [ 0, 0, 1 ] ) ).to.equal( foo ); + expect( frag.getNodeByPath( [ 0, 0, 2 ] ) ).to.equal( foo ); + expect( frag.getNodeByPath( [ 0, 0, 3 ] ) ).to.equal( image ); + } ); + + it( 'works fine with offsets', () => { + const bar = new Text( 'bar' ); + const foo = new Text( 'foo' ); + const bom = new Text( 'bom' ); + const bold = new Element( 'b', [], [ + bar + ] ); + const paragraph = new Element( 'paragraph', [], [ + foo, + bold, + bom + ] ); + const frag = new DocumentFragment( paragraph ); + + // foobarbom + + expect( frag.getNodeByPath( [ 0, 0 ] ) ).to.equal( foo ); + expect( frag.getNodeByPath( [ 0, 1 ] ) ).to.equal( foo ); + expect( frag.getNodeByPath( [ 0, 2 ] ) ).to.equal( foo ); + expect( frag.getNodeByPath( [ 0, 3 ] ) ).to.equal( bold ); + expect( frag.getNodeByPath( [ 0, 3, 0 ] ) ).to.equal( bar ); + expect( frag.getNodeByPath( [ 0, 3, 1 ] ) ).to.equal( bar ); + expect( frag.getNodeByPath( [ 0, 3, 2 ] ) ).to.equal( bar ); + expect( frag.getNodeByPath( [ 0, 3, 3 ] ) ).to.equal( null ); + expect( frag.getNodeByPath( [ 0, 4 ] ) ).to.equal( bom ); + expect( frag.getNodeByPath( [ 0, 5 ] ) ).to.equal( bom ); + expect( frag.getNodeByPath( [ 0, 6 ] ) ).to.equal( bom ); + expect( frag.getNodeByPath( [ 0, 7 ] ) ).to.equal( null ); } ); } ); } ); From 0eb6de7a011b3a83a5cea4c5494c599467a5840c Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Wed, 26 Jul 2017 08:58:59 +0200 Subject: [PATCH 3/4] Improved the tests. --- tests/model/documentfragment.js | 46 +++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/tests/model/documentfragment.js b/tests/model/documentfragment.js index 707815f20..75d15455b 100644 --- a/tests/model/documentfragment.js +++ b/tests/model/documentfragment.js @@ -326,6 +326,8 @@ describe( 'DocumentFragment', () => { } ); it( 'works fine with offsets', () => { + const abc = new Text( 'abc' ); + const xyz = new Text( 'xyz' ); const bar = new Text( 'bar' ); const foo = new Text( 'foo' ); const bom = new Text( 'bom' ); @@ -337,22 +339,34 @@ describe( 'DocumentFragment', () => { bold, bom ] ); - const frag = new DocumentFragment( paragraph ); - - // foobarbom - - expect( frag.getNodeByPath( [ 0, 0 ] ) ).to.equal( foo ); - expect( frag.getNodeByPath( [ 0, 1 ] ) ).to.equal( foo ); - expect( frag.getNodeByPath( [ 0, 2 ] ) ).to.equal( foo ); - expect( frag.getNodeByPath( [ 0, 3 ] ) ).to.equal( bold ); - expect( frag.getNodeByPath( [ 0, 3, 0 ] ) ).to.equal( bar ); - expect( frag.getNodeByPath( [ 0, 3, 1 ] ) ).to.equal( bar ); - expect( frag.getNodeByPath( [ 0, 3, 2 ] ) ).to.equal( bar ); - expect( frag.getNodeByPath( [ 0, 3, 3 ] ) ).to.equal( null ); - expect( frag.getNodeByPath( [ 0, 4 ] ) ).to.equal( bom ); - expect( frag.getNodeByPath( [ 0, 5 ] ) ).to.equal( bom ); - expect( frag.getNodeByPath( [ 0, 6 ] ) ).to.equal( bom ); - expect( frag.getNodeByPath( [ 0, 7 ] ) ).to.equal( null ); + const frag = new DocumentFragment( [ + abc, + paragraph, + xyz + ] ); + + // abcfoobarbomxyz + + expect( frag.getNodeByPath( [ 0 ] ), 1 ).to.equal( abc ); + expect( frag.getNodeByPath( [ 1 ] ), 2 ).to.equal( abc ); + expect( frag.getNodeByPath( [ 2 ] ), 3 ).to.equal( abc ); + expect( frag.getNodeByPath( [ 3 ] ), 4 ).to.equal( paragraph ); + expect( frag.getNodeByPath( [ 3, 0 ] ), 5 ).to.equal( foo ); + expect( frag.getNodeByPath( [ 3, 1 ] ), 6 ).to.equal( foo ); + expect( frag.getNodeByPath( [ 3, 2 ] ), 7 ).to.equal( foo ); + expect( frag.getNodeByPath( [ 3, 3 ] ), 8 ).to.equal( bold ); + expect( frag.getNodeByPath( [ 3, 3, 0 ] ), 9 ).to.equal( bar ); + expect( frag.getNodeByPath( [ 3, 3, 1 ] ), 10 ).to.equal( bar ); + expect( frag.getNodeByPath( [ 3, 3, 2 ] ), 11 ).to.equal( bar ); + expect( frag.getNodeByPath( [ 3, 3, 3 ] ), 12 ).to.equal( null ); + expect( frag.getNodeByPath( [ 3, 4 ] ), 13 ).to.equal( bom ); + expect( frag.getNodeByPath( [ 3, 5 ] ), 14 ).to.equal( bom ); + expect( frag.getNodeByPath( [ 3, 6 ] ), 15 ).to.equal( bom ); + expect( frag.getNodeByPath( [ 3, 7 ] ), 16 ).to.equal( null ); + expect( frag.getNodeByPath( [ 4 ] ), 17 ).to.equal( xyz ); + expect( frag.getNodeByPath( [ 5 ] ), 18 ).to.equal( xyz ); + expect( frag.getNodeByPath( [ 6 ] ), 19 ).to.equal( xyz ); + expect( frag.getNodeByPath( [ 7 ] ), 20 ).to.equal( null ); } ); } ); } ); From 8d0a7321811e0d432b3136908e969f16e4abf9ea Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 28 Jul 2017 12:04:27 +0200 Subject: [PATCH 4/4] Introduced a test which checks whether "Element#getNodeByPath()" works well. --- tests/controller/getselectedcontent.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/controller/getselectedcontent.js b/tests/controller/getselectedcontent.js index f156e5d24..bac043f61 100644 --- a/tests/controller/getselectedcontent.js +++ b/tests/controller/getselectedcontent.js @@ -309,6 +309,26 @@ describe( 'Delete utils', () => { expect( content ) .to.equal( 'bcxde' ); } ); + + // See: https://github.com/ckeditor/ckeditor5-engine/pull/1043#issuecomment-318012286 + it( 'ensures that elements are retrieved by indexes instead of offsets', () => { + doc.schema.allow( { name: '$text', inside: '$root' } ); + doc.schema.allow( { name: '$text', inside: 'quote' } ); + + setData( doc, + 'foo' + + '' + + '' + + 'b[ar' + + '' + + 'bo]m' + + '' + ); + + const content = stringify( getSelectedContent( doc.selection ) ); + expect( content ) + .to.equal( 'arbo' ); + } ); } ); } ); } );