From 45a46305cecaed0f6c521ba1eb5fc023e0ee52a2 Mon Sep 17 00:00:00 2001 From: Dmitri Pisarev Date: Thu, 16 Aug 2018 14:02:08 +0300 Subject: [PATCH 1/5] Fix: made `startsWithFiller` to use safe `isText` check instead of `instanceof` Using `domNode instanceof Text` is not safe and may cause issues e.g. across iframes. --- src/view/filler.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/view/filler.js b/src/view/filler.js index 25b1c55c4..02bab3800 100644 --- a/src/view/filler.js +++ b/src/view/filler.js @@ -6,6 +6,7 @@ /* globals window, Text */ import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; +import isText from '@ckeditor/ckeditor5-utils/src/dom/istext'; /** * Set of utils related to block and inline fillers handling. @@ -84,7 +85,7 @@ for ( let i = 0; i < INLINE_FILLER_LENGTH; i++ ) { * @returns {Boolean} True if the text node starts with the {@link module:engine/view/filler~INLINE_FILLER inline filler}. */ export function startsWithFiller( domNode ) { - return ( domNode instanceof Text ) && ( domNode.data.substr( 0, INLINE_FILLER_LENGTH ) === INLINE_FILLER ); + return isText( domNode ) && ( domNode.data.substr( 0, INLINE_FILLER_LENGTH ) === INLINE_FILLER ); } /** From b0e7adc87985d7e79985189076c07c8a2c9f3944 Mon Sep 17 00:00:00 2001 From: Dmitri Pisarev Date: Thu, 16 Aug 2018 14:11:57 +0300 Subject: [PATCH 2/5] Remove Text from globals --- src/view/filler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/view/filler.js b/src/view/filler.js index 02bab3800..a6e6ef2cc 100644 --- a/src/view/filler.js +++ b/src/view/filler.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md. */ -/* globals window, Text */ +/* globals window */ import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import isText from '@ckeditor/ckeditor5-utils/src/dom/istext'; From ad2bd3c8511396dcaabf0f5bff1d792d23c58cfc Mon Sep 17 00:00:00 2001 From: Dmitri Pisarev Date: Thu, 16 Aug 2018 14:40:28 +0300 Subject: [PATCH 3/5] Add a test --- tests/view/filler.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/view/filler.js b/tests/view/filler.js index 96223e138..88cb3d40c 100644 --- a/tests/view/filler.js +++ b/tests/view/filler.js @@ -111,6 +111,14 @@ describe( 'filler', () => { expect( isInlineFiller( node ) ).to.be.false; } ); + + it( 'should be true for inline filler from inside iframe', () => { + const iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + const node = iframe.contentDocument.createTextNode( INLINE_FILLER ); + + expect( isInlineFiller( node ) ).to.be.true; + } ); } ); describe( 'isBlockFiller', () => { From 6b1ff3dddc178b853c89ad8be08330be99917fba Mon Sep 17 00:00:00 2001 From: Dmitri Pisarev Date: Thu, 16 Aug 2018 14:52:55 +0300 Subject: [PATCH 4/5] Linter --- tests/view/filler.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/view/filler.js b/tests/view/filler.js index 88cb3d40c..a26c0ead2 100644 --- a/tests/view/filler.js +++ b/tests/view/filler.js @@ -111,10 +111,10 @@ describe( 'filler', () => { expect( isInlineFiller( node ) ).to.be.false; } ); - + it( 'should be true for inline filler from inside iframe', () => { - const iframe = document.createElement('iframe'); - document.body.appendChild(iframe); + const iframe = document.createElement( 'iframe' ); + document.body.appendChild( iframe ); const node = iframe.contentDocument.createTextNode( INLINE_FILLER ); expect( isInlineFiller( node ) ).to.be.true; From a7e3a45452d405bc3d236ea96df3bb727a9de6c0 Mon Sep 17 00:00:00 2001 From: Dmitri Pisarev Date: Thu, 16 Aug 2018 22:02:41 +0300 Subject: [PATCH 5/5] Remove iframe from document after the test --- tests/view/filler.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/view/filler.js b/tests/view/filler.js index a26c0ead2..0a40dd38d 100644 --- a/tests/view/filler.js +++ b/tests/view/filler.js @@ -118,6 +118,8 @@ describe( 'filler', () => { const node = iframe.contentDocument.createTextNode( INLINE_FILLER ); expect( isInlineFiller( node ) ).to.be.true; + + document.body.removeChild( iframe ); } ); } );