From 9807e4f1fd71e8c0186634709b6d97c384f86b7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 19 Oct 2017 17:24:43 +0200 Subject: [PATCH 1/3] Blocking inline fillers on elements with contenteditable=false. --- src/view/renderer.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/view/renderer.js b/src/view/renderer.js index c2fa297bf..ba2e64807 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -381,6 +381,11 @@ export default class Renderer { return false; } + // Block adding inline filler inside elements with contenteditable=false. + if ( _isNotEditable( selectionParent ) ) { + return false; + } + // We have block filler, we do not need inline one. if ( selectionOffset === selectionParent.getFillerOffset() ) { return false; @@ -708,3 +713,18 @@ export default class Renderer { } mix( Renderer, ObservableMixin ); + +// Checks if contents inside provided element are not editable. +// +// @private +// @param {module:engine/view/element~Element} element +// @returns {Boolean} +function _isNotEditable( element ) { + if ( element.getAttribute( 'contenteditable' ) === false ) { + return true; + } + + const parent = element.findAncestor( element => element.hasAttribute( 'contenteditable' ) ); + + return parent && !parent.getAttribute( 'contenteditable' ); +} From b456008b9bd817c596118d0aac630a837dacae40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Fri, 20 Oct 2017 16:04:02 +0200 Subject: [PATCH 2/3] Added test checking if inline filler is not rendered inside non editable content. --- src/view/renderer.js | 5 +++-- tests/view/renderer.js | 44 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/view/renderer.js b/src/view/renderer.js index ba2e64807..bc8de5525 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -382,6 +382,7 @@ export default class Renderer { } // Block adding inline filler inside elements with contenteditable=false. + // https://github.com/ckeditor/ckeditor5-engine/issues/1170 if ( _isNotEditable( selectionParent ) ) { return false; } @@ -720,11 +721,11 @@ mix( Renderer, ObservableMixin ); // @param {module:engine/view/element~Element} element // @returns {Boolean} function _isNotEditable( element ) { - if ( element.getAttribute( 'contenteditable' ) === false ) { + if ( element.getAttribute( 'contenteditable' ) == 'false' ) { return true; } const parent = element.findAncestor( element => element.hasAttribute( 'contenteditable' ) ); - return parent && !parent.getAttribute( 'contenteditable' ); + return parent && parent.getAttribute( 'contenteditable' ) == 'false'; } diff --git a/tests/view/renderer.js b/tests/view/renderer.js index 7fc1ef786..64fb05969 100644 --- a/tests/view/renderer.js +++ b/tests/view/renderer.js @@ -410,6 +410,50 @@ describe( 'Renderer', () => { expect( viewRoot ).to.be.ok; } ); + it( 'should not add filler when inside contenteditable=false parent', () => { + const { view: viewP, selection: newSelection } = parse( + 'foo[]bar' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + expect( domRoot.childNodes.length ).to.equal( 1 ); + expect( domRoot.childNodes[ 0 ].tagName.toLowerCase() ).to.equal( 'p' ); + + const domP = domRoot.childNodes[ 0 ]; + + expect( domP.childNodes.length ).to.equal( 3 ); + expect( domP.childNodes[ 0 ].data ).to.equal( 'foo' ); + expect( domP.childNodes[ 2 ].data ).to.equal( 'bar' ); + expect( domP.childNodes[ 1 ].tagName.toLowerCase() ).to.equal( 'b' ); + expect( domP.childNodes[ 1 ].childNodes.length ).to.equal( 0 ); + } ); + + it( 'should not add filler when inside contenteditable=false ancestor', () => { + const { view: viewP, selection: newSelection } = parse( + 'foo[]bar' ); + + viewRoot.appendChildren( viewP ); + selection.setTo( newSelection ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + expect( domRoot.childNodes.length ).to.equal( 1 ); + expect( domRoot.childNodes[ 0 ].tagName.toLowerCase() ).to.equal( 'p' ); + + const domP = domRoot.childNodes[ 0 ]; + + expect( domP.childNodes.length ).to.equal( 3 ); + expect( domP.childNodes[ 0 ].data ).to.equal( 'foo' ); + expect( domP.childNodes[ 2 ].data ).to.equal( 'bar' ); + expect( domP.childNodes[ 1 ].tagName.toLowerCase() ).to.equal( 'b' ); + expect( domP.childNodes[ 1 ].childNodes.length ).to.equal( 0 ); + } ); + it( 'should add and remove inline filler in case

foo[]bar

', () => { const domSelection = document.getSelection(); From 8832628dac15e936e0aafc3cf3c268523f9dac5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Fri, 20 Oct 2017 16:16:22 +0200 Subject: [PATCH 3/3] Small refactoring, docs fixes. --- src/view/renderer.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/view/renderer.js b/src/view/renderer.js index bc8de5525..f96130075 100644 --- a/src/view/renderer.js +++ b/src/view/renderer.js @@ -381,9 +381,9 @@ export default class Renderer { return false; } - // Block adding inline filler inside elements with contenteditable=false. + // Prevent adding inline filler inside elements with contenteditable=false. // https://github.com/ckeditor/ckeditor5-engine/issues/1170 - if ( _isNotEditable( selectionParent ) ) { + if ( !_isEditable( selectionParent ) ) { return false; } @@ -715,17 +715,17 @@ export default class Renderer { mix( Renderer, ObservableMixin ); -// Checks if contents inside provided element are not editable. +// Checks if provided element is editable. // // @private // @param {module:engine/view/element~Element} element // @returns {Boolean} -function _isNotEditable( element ) { +function _isEditable( element ) { if ( element.getAttribute( 'contenteditable' ) == 'false' ) { - return true; + return false; } const parent = element.findAncestor( element => element.hasAttribute( 'contenteditable' ) ); - return parent && parent.getAttribute( 'contenteditable' ) == 'false'; + return !parent || parent.getAttribute( 'contenteditable' ) == 'true'; }