diff --git a/src/view/domconverter.js b/src/view/domconverter.js index f6d09e882..c9861f77f 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -7,7 +7,7 @@ * @module engine/view/domconverter */ -/* globals document, Node, NodeFilter */ +/* globals document, Node, NodeFilter, Text */ import ViewText from './text'; import ViewElement from './element'; @@ -956,10 +956,10 @@ export default class DomConverter { * @private */ _processDataFromDomText( node ) { - let data = getDataWithoutFiller( node ); + let data = node.data; if ( _hasDomParentOfType( node, this.preElements ) ) { - return data; + return getDataWithoutFiller( node ); } // Change all consecutive whitespace characters (from the [ \n\t\r] set – @@ -978,9 +978,16 @@ export default class DomConverter { } // If next text node does not exist remove space character from the end of this text node. - if ( !nextNode ) { + if ( !nextNode && !startsWithFiller( node ) ) { data = data.replace( / $/, '' ); } + + // At the beginning and end of a block element, Firefox inserts normal space +
instead of non-breaking space. + // This means that the text node starts/end with normal space instead of non-breaking space. + // This causes a problem because the normal space would be removed in `.replace` calls above. To prevent that, + // the inline filler is removed only after the data is initially processed (by the `.replace` above). See ckeditor5#692. + data = getDataWithoutFiller( new Text( data ) ); + // At this point we should have removed all whitespaces from DOM text data. // Now we have to change   chars, that were in DOM text data because of rendering reasons, to spaces. diff --git a/tests/tickets/ckeditor5-692.js b/tests/tickets/ckeditor5-692.js new file mode 100644 index 000000000..b5a3ae32c --- /dev/null +++ b/tests/tickets/ckeditor5-692.js @@ -0,0 +1,87 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import MutationObserver from '../../src/view/observer/mutationobserver'; +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import { getData as getModelData, setData as setModelData } from '../../src/dev-utils/model'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold.js'; +import { getData as getViewData } from '../../src/dev-utils/view'; +import { isInlineFiller } from '../../src/view/filler'; +import Input from '@ckeditor/ckeditor5-typing/src/input'; + +/* globals document */ + +describe( 'Bug ckeditor5#692', () => { + let editorElement, editor, mutationObserver, view, domEditor; + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ Paragraph, Bold, Input ] + } ) + .then( newEditor => { + editor = newEditor; + view = editor.editing.view; + mutationObserver = view.getObserver( MutationObserver ); + domEditor = editor.ui.view.editableElement; + } ); + } ); + + afterEach( () => { + document.body.removeChild( editorElement ); + + return editor.destroy(); + } ); + + describe( 'DomConverter', () => { + // https://github.com/ckeditor/ckeditor5/issues/692 Scenario 1. + it( 'should handle space after inline filler at the end of container', () => { + setModelData( editor.model, 'foo[]' ); + + // Create Bold attribute at the end of paragraph. + editor.execute( 'bold' ); + + expect( getModelData( editor.model ) ).to.equal( 'foo<$text bold="true">[]' ); + + const domParagraph = domEditor.childNodes[ 0 ]; + const textNode = domParagraph.childNodes[ 1 ].childNodes[ 0 ]; + + expect( isInlineFiller( textNode ) ).to.be.true; + + // Add space inside the strong's text node. + textNode.data += ' '; + mutationObserver.flush(); + + expect( getModelData( editor.model ) ).to.equal( 'foo<$text bold="true"> []' ); + expect( getViewData( editor.editing.view ) ).to.equal( '

foo {}

' ); + } ); + + // https://github.com/ckeditor/ckeditor5/issues/692 Scenario 2. + it( 'should handle space after inline filler at the end of container', () => { + setModelData( editor.model, '[]foo' ); + + // Create Bold attribute at the end of paragraph. + editor.execute( 'bold' ); + + expect( getModelData( editor.model ) ).to.equal( '<$text bold="true">[]foo' ); + + const domParagraph = domEditor.childNodes[ 0 ]; + const textNode = domParagraph.childNodes[ 0 ].childNodes[ 0 ]; + + expect( isInlineFiller( textNode ) ).to.be.true; + + // Add space inside the strong's text node. + textNode.data += ' '; + mutationObserver.flush(); + + expect( getModelData( editor.model ) ).to.equal( '<$text bold="true"> []foo' ); + expect( getViewData( editor.editing.view ) ).to.equal( '

{}foo

' ); + } ); + } ); +} ); diff --git a/tests/view/domconverter/dom-to-view.js b/tests/view/domconverter/dom-to-view.js index a30194787..558cde1ca 100644 --- a/tests/view/domconverter/dom-to-view.js +++ b/tests/view/domconverter/dom-to-view.js @@ -525,6 +525,30 @@ describe( 'DomConverter', () => { // See also whitespace-handling-integration.js. // } ); + + describe( 'clearing auto filler', () => { + it( 'should remove inline filler when converting dom to view', () => { + const text = document.createTextNode( INLINE_FILLER + 'foo' ); + const view = converter.domToView( text ); + + expect( view.data ).to.equal( 'foo' ); + } ); + + // See https://github.com/ckeditor/ckeditor5/issues/692. + it( 'should not remove space after inline filler if previous node nor next node does not exist', () => { + const text = document.createTextNode( INLINE_FILLER + ' ' ); + const view = converter.domToView( text ); + + expect( view.data ).to.equal( ' ' ); + } ); + + it( 'should convert non breaking space to normal space after inline filler', () => { + const text = document.createTextNode( INLINE_FILLER + '\u00A0' ); + const view = converter.domToView( text ); + + expect( view.data ).to.equal( ' ' ); + } ); + } ); } ); describe( 'domChildrenToView', () => { diff --git a/tests/view/observer/mutationobserver.js b/tests/view/observer/mutationobserver.js index d7aef2bfa..16dc59eab 100644 --- a/tests/view/observer/mutationobserver.js +++ b/tests/view/observer/mutationobserver.js @@ -269,6 +269,112 @@ describe( 'MutationObserver', () => { expect( lastMutations[ 0 ].newText ).to.equal( 'xy' ); } ); + // https://github.com/ckeditor/ckeditor5/issues/692 Scenario 1. + it( 'should handle space after inline filler at the end of container', () => { + const { view: viewContainer, selection } = parse( + '' + + 'foo' + + '[]' + + '' + ); + + view.change( writer => { + viewRoot.appendChildren( viewContainer ); + writer.setSelection( selection ); + } ); + + // Appended container is third in the tree. + const container = domEditor.childNodes[ 2 ]; + const inlineFiller = container.childNodes[ 1 ].childNodes[ 0 ]; + + inlineFiller.data += ' '; + + mutationObserver.flush(); + + expect( lastMutations.length ).to.equal( 1 ); + expect( lastMutations[ 0 ].type ).to.equal( 'children' ); + expect( lastMutations[ 0 ].oldChildren.length ).to.equal( 0 ); + expect( lastMutations[ 0 ].newChildren.length ).to.equal( 1 ); + expect( lastMutations[ 0 ].newChildren[ 0 ].is( 'text' ) ).to.be.true; + expect( lastMutations[ 0 ].newChildren[ 0 ].data ).to.equal( ' ' ); + expect( lastMutations[ 0 ].node ).to.equal( selection.getFirstPosition().parent ); + } ); + + // https://github.com/ckeditor/ckeditor5/issues/692 Scenario 3. + it( 'should handle space after inline filler at the end of container #2', () => { + const { view: viewContainer, selection } = parse( + '' + + 'foo' + + 'bar' + + '[]' + + '' + ); + + view.change( writer => { + viewRoot.appendChildren( viewContainer ); + writer.setSelection( selection ); + } ); + + // Appended container is third in the tree. + const container = domEditor.childNodes[ 2 ]; + const inlineFiller = container.childNodes[ 2 ]; + + inlineFiller.data += ' '; + + mutationObserver.flush(); + + expect( lastMutations.length ).to.equal( 1 ); + expect( lastMutations[ 0 ].type ).to.equal( 'children' ); + expect( lastMutations[ 0 ].oldChildren.length ).to.equal( 2 ); + expect( lastMutations[ 0 ].newChildren.length ).to.equal( 3 ); + + // Foo and attribute is removed and reinserted. + expect( lastMutations[ 0 ].oldChildren[ 0 ].is( 'text' ) ).to.be.true; + expect( lastMutations[ 0 ].oldChildren[ 0 ].data ).to.equal( 'foo' ); + expect( lastMutations[ 0 ].newChildren[ 0 ].is( 'text' ) ).to.be.true; + expect( lastMutations[ 0 ].newChildren[ 0 ].data ).to.equal( 'foo' ); + + expect( lastMutations[ 0 ].oldChildren[ 1 ].is( 'attributeElement' ) ).to.be.true; + expect( lastMutations[ 0 ].oldChildren[ 1 ].name ).to.equal( 'b' ); + expect( lastMutations[ 0 ].newChildren[ 1 ].is( 'attributeElement' ) ).to.be.true; + expect( lastMutations[ 0 ].newChildren[ 1 ].name ).to.equal( 'b' ); + + expect( lastMutations[ 0 ].newChildren[ 2 ].is( 'text' ) ).to.be.true; + expect( lastMutations[ 0 ].newChildren[ 2 ].data ).to.equal( ' ' ); + expect( lastMutations[ 0 ].node ).to.equal( selection.getFirstPosition().parent ); + } ); + + // https://github.com/ckeditor/ckeditor5/issues/692 Scenario 2. + it( 'should handle space after inline filler at the beginning of container', () => { + const { view: viewContainer, selection } = parse( + '' + + '[]' + + 'foo' + + '' + ); + + view.change( writer => { + viewRoot.appendChildren( viewContainer ); + writer.setSelection( selection ); + } ); + + // Appended container is third in the tree. + const container = domEditor.childNodes[ 2 ]; + const inlineFiller = container.childNodes[ 0 ].childNodes[ 0 ]; + + inlineFiller.data += ' '; + + mutationObserver.flush(); + + expect( lastMutations.length ).to.equal( 1 ); + expect( lastMutations[ 0 ].type ).to.equal( 'children' ); + expect( lastMutations[ 0 ].oldChildren.length ).to.equal( 0 ); + expect( lastMutations[ 0 ].newChildren.length ).to.equal( 1 ); + expect( lastMutations[ 0 ].newChildren[ 0 ].is( 'text' ) ).to.be.true; + expect( lastMutations[ 0 ].newChildren[ 0 ].data ).to.equal( ' ' ); + expect( lastMutations[ 0 ].node ).to.equal( selection.getFirstPosition().parent ); + } ); + it( 'should have no block filler in mutation', () => { viewRoot.appendChildren( parse( '' ) ); @@ -359,7 +465,7 @@ describe( 'MutationObserver', () => { mutationObserver.flush(); - // There was onlu P2 change. P1 must be ignored. + // There was only P2 change. P1 must be ignored. const viewP2 = viewRoot.getChild( 1 ); expect( lastMutations.length ).to.equal( 1 ); expect( lastMutations[ 0 ].node ).to.equal( viewP2 );