Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/1009: Element#getNodeByPath() should expect offsets not indexes #1043

Merged
merged 6 commits into from
Jul 28, 2017
Merged

T/1009: Element#getNodeByPath() should expect offsets not indexes #1043

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a7765d2
"model.Element#getNodeByPath()" works with offsets instead of indexes.
Jul 25, 2017
8bc50cb
"model.DocumentFragment#getNodeByPath()" works with offsets instead o…
Jul 25, 2017
0eb6de7
Improved the tests.
Jul 26, 2017
76f5adb
Merge branch 'master' into t/1009
Jul 26, 2017
ec9d8d6
Merge branch 'master' into t/1009
Jul 28, 2017
8d0a732
Introduced a test which checks whether "Element#getNodeByPath()" work…
Jul 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/model/documentfragment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/model/element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
38 changes: 36 additions & 2 deletions tests/model/documentfragment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

// <paragraph>foo<bold>bar</bold>bom</paragraph>

expect( frag.getNodeByPath( [ 0, 0 ] ) ).to.equal( foo );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not good when all tests start with 0 offset. E.g. if we'd implement this method recursively this would pass while there still might be a bug in DF itself.

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 );
} );
} );
} );
37 changes: 35 additions & 2 deletions tests/model/element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
] );

// <paragraph>foo<bold>bar</bold>bom</paragraph>

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 );
} );
} );

Expand Down