Procházet zdrojové kódy

Merge pull request #457 from ckeditor/t/452

t/452: Renderer should work fine without DOM.
Piotrek Koszuliński před 9 roky
rodič
revize
cb031ea98a

+ 3 - 2
packages/ckeditor5-engine/src/model/selection.js

@@ -498,9 +498,10 @@ export default class Selection {
 
 			// ...look for a first character node in that range and take attributes from it.
 			for ( let item of range ) {
-				if ( item.type == 'TEXT' ) {
+				// This is not an optimal solution because of https://github.com/ckeditor/ckeditor5-engine/issues/454.
+				// It can be done better by using `break;` instead of checking `attrs === null`.
+				if ( item.type == 'TEXT' && attrs === null ) {
 					attrs = item.item.getAttributes();
-					break;
 				}
 			}
 		} else {

+ 5 - 1
packages/ckeditor5-engine/src/view/element.js

@@ -196,7 +196,11 @@ export default class Element extends Node {
 			yield 'style';
 		}
 
-		yield* this._attrs.keys();
+		// This is not an optimal solution because of https://github.com/ckeditor/ckeditor5-engine/issues/454.
+		// It can be simplified to `yield* this._attrs.keys();`.
+		for ( let key of this._attrs.keys() ) {
+			yield key;
+		}
 	}
 
 	/**

+ 14 - 3
packages/ckeditor5-engine/src/view/renderer.js

@@ -167,6 +167,7 @@ export default class Renderer {
 
 			if ( this._needAddInlineFiller() ) {
 				this._inlineFillerPosition = this.selection.getFirstPosition();
+				// Do not use `markToSync` so it will be added even if the parent is already added.
 				this.markedChildren.add( this._inlineFillerPosition.parent );
 			} else {
 				this._inlineFillerPosition = null;
@@ -273,6 +274,11 @@ export default class Renderer {
 		const selectionParent = selectionPosition.parent;
 		const selectionOffset = selectionPosition.offset;
 
+		// If there is no DOM root we do not care about fillers.
+		if ( !this.domConverter.getCorrespondingDomElement( selectionParent.getRoot() ) ) {
+			return false;
+		}
+
 		if ( !( selectionParent instanceof ViewElement ) ) {
 			return false;
 		}
@@ -356,10 +362,10 @@ export default class Renderer {
 		const expectedDomChildren = Array.from( domConverter.viewChildrenToDom( viewElement, domDocument, { bind: true } ) );
 
 		if ( filler && filler.parent == viewElement ) {
-			const expectedNoteAfterFiller = expectedDomChildren[ filler.offset ];
+			const expectedNodeAfterFiller = expectedDomChildren[ filler.offset ];
 
-			if ( expectedNoteAfterFiller instanceof Text ) {
-				expectedNoteAfterFiller.data = INLINE_FILLER + expectedNoteAfterFiller.data;
+			if ( expectedNodeAfterFiller instanceof Text ) {
+				expectedNodeAfterFiller.data = INLINE_FILLER + expectedNodeAfterFiller.data;
 			} else {
 				expectedDomChildren.splice( filler.offset, 0, domDocument.createTextNode( INLINE_FILLER ) );
 			}
@@ -411,6 +417,11 @@ export default class Renderer {
 		}
 
 		const domRoot = this.domConverter.getCorrespondingDomElement( this.focusedEditable );
+
+		if ( !domRoot ) {
+			return;
+		}
+
 		const domSelection = domRoot.ownerDocument.defaultView.getSelection();
 		const oldViewSelection = domSelection && this.domConverter.domSelectionToView( domSelection );
 

+ 2 - 0
packages/ckeditor5-engine/tests/editingcontroller.js

@@ -131,6 +131,8 @@ describe( 'EditingController', () => {
 		} );
 
 		beforeEach( () => {
+			// Note: The below code is highly overcomplicated due to #455.
+
 			model.selection.removeAllRanges();
 			modelRoot.removeChildren( 0, modelRoot.getChildCount() );
 

+ 17 - 0
packages/ckeditor5-engine/tests/view/renderer.js

@@ -279,6 +279,23 @@ describe( 'Renderer', () => {
 			expect( domRoot.childNodes[ 0 ].tagName ).to.equal( 'P' );
 		} );
 
+		it( 'should not care about filler if there is no DOM', () => {
+			const { view: viewP, selection: newSelection } = parse(
+				'<container:p>foo<attribute:b>[]</attribute:b>bar</container:p>' );
+
+			const viewRoot = new ViewElement( 'p' );
+			viewRoot.appendChildren( viewP );
+			selection.setTo( newSelection );
+
+			renderer.focusedEditable = viewRoot;
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			// Expect no error on render.
+			expect( viewRoot ).to.be.ok;
+		} );
+
 		it( 'should add and remove inline filler in case <p>foo<b>[]</b>bar</p>', () => {
 			const domSelection = document.getSelection();