Explorar o código

Fixes, fixes, fixes.

Piotr Jasiun %!s(int64=9) %!d(string=hai) anos
pai
achega
0f253d96e6

+ 6 - 2
packages/ckeditor5-engine/src/treeview/attributeelement.js

@@ -75,7 +75,7 @@ export default class AttributeElement extends Element {
 
 	needsFiller() {
 		// <b>foo</b> does not need filler
-		if ( !this.getChildCount() ) {
+		if ( this.getChildCount() ) {
 			return false;
 		}
 
@@ -89,6 +89,10 @@ export default class AttributeElement extends Element {
 			element = element.parent;
 		}
 
-		return this.getChildCount() === 1;
+		if ( element.getChildCount() > 1 ) {
+			return false;
+		}
+
+		return 0;
 	}
 }

+ 44 - 11
packages/ckeditor5-engine/src/treeview/domconverter.js

@@ -8,6 +8,7 @@
 import ViewText from './text.js';
 import ViewElement from './element.js';
 import ViewPosition from './position.js';
+import ViewRange from './range.js';
 import ViewSelection from './selection.js';
 import ViewDocumentFragment from './documentfragment.js';
 
@@ -165,6 +166,10 @@ export default class DomConverter {
 
 			offset++;
 		}
+
+		if ( fillerPositionOffset === offset ) {
+			yield this.blockFillerCreator( domDocument );
+		}
 	}
 
 	/**
@@ -254,9 +259,11 @@ export default class DomConverter {
 			return this.getCorrespondingViewElement( domNode );
 		} else if ( domNode instanceof DocumentFragment ) {
 			return this.getCorrespondingViewDocumentFragment( domNode );
-		} else {
+		} else if ( domNode instanceof Text ) {
 			return this.getCorrespondingViewText( domNode );
 		}
+
+		return undefined;
 	}
 
 	/**
@@ -314,7 +321,13 @@ export default class DomConverter {
 			const viewElement = this.getCorrespondingViewElement( previousSibling );
 
 			if ( viewElement ) {
-				return viewElement.getNextSibling();
+				const nextSibling = viewElement.getNextSibling();
+
+				if ( nextSibling instanceof ViewText ) {
+					return viewElement.getNextSibling();
+				} else {
+					return null;
+				}
 			}
 		}
 		// Try to use parent to find the corresponding text node.
@@ -322,7 +335,13 @@ export default class DomConverter {
 			const viewElement = this.getCorrespondingViewElement( domText.parentNode );
 
 			if ( viewElement ) {
-				return viewElement.getChild( 0 );
+				const firstChild = viewElement.getChild( 0 );
+
+				if ( firstChild instanceof ViewText ) {
+					return firstChild;
+				} else {
+					return null;
+				}
 			}
 		}
 
@@ -419,14 +438,14 @@ export default class DomConverter {
 
 			if ( viewPosition.offset === 0 ) {
 				domParent = this.getCorrespondingDom( viewPosition.parent );
-				domAfter = parent.childNodes[ 0 ];
+				domAfter = domParent.childNodes[ 0 ];
 			} else {
-				domBefore = this.getCorrespondingDom( viewPosition.nodeBefore() );
+				domBefore = this.getCorrespondingDom( viewPosition.nodeBefore );
 				domParent = domBefore.parentNode;
 				domAfter = domBefore.nextSibling;
 			}
 
-			if ( domAfter instanceof ViewText && this.isInlineFiller( domAfter ) ) {
+			if ( domAfter instanceof Text && this.startsWithFiller( domAfter ) ) {
 				return { parent: domAfter, offset: INLINE_FILLER_SIZE };
 			}
 
@@ -459,12 +478,18 @@ export default class DomConverter {
 			if ( domOffset === 0 ) {
 				const viewParent = this.getCorrespondingView( domParent );
 
-				return new ViewPosition( viewParent, 0 );
+				if ( viewParent ) {
+					return new ViewPosition( viewParent, 0 );
+				}
 			} else {
-				const viewBefore = this.getCorrespondingView( domParent.childNodes[ domOffset ] );
+				const viewBefore = this.getCorrespondingView( domParent.childNodes[ domOffset - 1 ] );
 
-				return new ViewPosition( viewBefore.parent, viewBefore.getIndex() + 1 );
+				if ( viewBefore ) {
+					return new ViewPosition( viewBefore.parent, viewBefore.getIndex() + 1 );
+				}
 			}
+
+			return undefined;
 		}
 	}
 
@@ -483,7 +508,11 @@ export default class DomConverter {
 		const viewStart = this.domPositionToView( domRange.startContainer, domRange.startOffset );
 		const viewEnd = this.domPositionToView( domRange.endContainer, domRange.endOffset );
 
-		return new Range( viewStart, viewEnd );
+		if ( viewStart && viewEnd ) {
+			return new ViewRange( viewStart, viewEnd );
+		}
+
+		return undefined;
 	}
 
 	domSelectionToView( domSelection ) {
@@ -493,7 +522,9 @@ export default class DomConverter {
 			const domRange = domSelection.getRangeAt( i );
 			const viewRange = this.domRangeToView( domRange );
 
-			viewSelection.addRange( viewRange );
+			if ( viewRange ) {
+				viewSelection.addRange( viewRange );
+			}
 		}
 
 		return viewSelection;
@@ -519,4 +550,6 @@ function indexOf( domNode ) {
 		domNode = domNode.previousSibling;
 		index++;
 	}
+
+	return index;
 }

+ 9 - 5
packages/ckeditor5-engine/src/treeview/observer/mutationobserver.js

@@ -117,19 +117,21 @@ export default class MutationObserver extends Observer {
 			return;
 		}
 
+		const domConverter = this.domConverter;
+
 		// Use map and set for deduplication.
 		const mutatedTexts = new Map();
 		const mutatedElements = new Set();
 
 		// Assume that all elements are in the same document.
 		const domSelection = domMutations[ 0 ].target.ownerDocument.defaultView.getSelection();
-		const viewSelection = this.domConverter.domSelectionToView( domSelection );
+		const viewSelection = domConverter.domSelectionToView( domSelection );
 
 		// Handle `childList` mutations first, so we will be able to check if the `characterData` mutation is in the
 		// element with changed structure anyway.
 		for ( let mutation of domMutations ) {
 			if ( mutation.type === 'childList' ) {
-				const element = this.domConverter.getCorrespondingViewElement( mutation.target );
+				const element = domConverter.getCorrespondingViewElement( mutation.target );
 
 				if ( element ) {
 					mutatedElements.add( element );
@@ -140,7 +142,7 @@ export default class MutationObserver extends Observer {
 		// Handle `characterData` mutations later, when we have the full list of nodes which changed structure.
 		for ( let mutation of domMutations ) {
 			if ( mutation.type === 'characterData' ) {
-				const text = this.domConverter.getCorrespondingViewText( mutation.target );
+				const text = domConverter.getCorrespondingViewText( mutation.target );
 
 				if ( text && !mutatedElements.has( text.parent ) ) {
 					// Use text as a key, for deduplication. If there will be another mutation on the same text element
@@ -152,6 +154,8 @@ export default class MutationObserver extends Observer {
 						node: text,
 						selection: viewSelection
 					} );
+				} else if ( !text && domConverter.startsWithFiller( mutation.target ) ) {
+					mutatedElements.add( domConverter.getCorrespondingViewElement( mutation.target.parentNode ) );
 				}
 			}
 		}
@@ -168,9 +172,9 @@ export default class MutationObserver extends Observer {
 		}
 
 		for ( let viewElement of mutatedElements ) {
-			const domElement = this.domConverter.getCorrespondingDomElement( viewElement );
+			const domElement = domConverter.getCorrespondingDomElement( viewElement );
 			const viewChildren = viewElement.getChildren();
-			const newViewChildren = this.domConverter.domChildrenToView( domElement );
+			const newViewChildren = domConverter.domChildrenToView( domElement );
 
 			this.renderer.markToSync( 'CHILDREN', viewElement );
 			viewMutations.push( {

+ 25 - 19
packages/ckeditor5-engine/src/treeview/observer/selectionobserver.js

@@ -29,31 +29,37 @@ export default class SelectionObserver extends Observer {
 		if ( this.documents.has( domDocument ) ) {
 			return;
 		}
+		this._handleSelectionChange();
 
-		domDocument.addEventListener( 'selectionchange', () => {
-			if ( !this.isEnabled ) {
-				return;
-			}
+		domDocument.addEventListener( 'selectionchange', () => this._handleSelectionChange( domDocument ) );
 
-			// Ensure the mutation event will be before selection event on all browsers.
-			this.mutationObserver.flush();
+		this.documents.add( domDocument );
+	}
 
-			// If there were mutations then the view will be re-rendered by the mutations observer and selection
-			// will be updated, so selection will be equal and event will not be fires, as expected.
-			const domSelection = domDocument.defaultView.getSelection();
-			const newViewSelection = this.domConverter.domSelectionToView( domSelection );
+	_handleSelectionChange( domDocument ) {
+		if ( !this.isEnabled ) {
+			return;
+		}
 
-			if ( this.selection.isEqual( newViewSelection ) ) {
-				return;
-			}
+		// Ensure the mutation event will be before selection event on all browsers.
+		this.mutationObserver.flush();
 
-			// Should be fired only when selection change was the only document change.
-			this.treeView.fire( 'selectionchange', {
-				oldSelection: this.selection,
-				newSelection: newViewSelection
-			} );
+		// If there were mutations then the view will be re-rendered by the mutations observer and selection
+		// will be updated, so selection will be equal and event will not be fires, as expected.
+		const domSelection = domDocument.defaultView.getSelection();
+		const newViewSelection = this.domConverter.domSelectionToView( domSelection );
 
-			this.treeView.render();
+		if ( this.selection.isEqual( newViewSelection ) ) {
+			return;
+		}
+
+		// Should be fired only when selection change was the only document change.
+		this.treeView.fire( 'selectionchange', {
+			oldSelection: this.selection,
+			newSelection: newViewSelection,
+			domSelection: domSelection
 		} );
+
+		this.treeView.render();
 	}
 }

+ 38 - 26
packages/ckeditor5-engine/src/treeview/renderer.js

@@ -5,14 +5,14 @@
 
 'use strict';
 
-import diff from '../../utils/diff.js';
 import ViewText from './text.js';
-
 import ViewElement from './element.js';
 import ViewPosition from './position.js';
-
 import { INLINE_FILLER, INLINE_FILLER_SIZE } from './domconverter.js';
 
+import SelectionObserver from './observer/selectionobserver.js';
+
+import diff from '../../utils/diff.js';
 import CKEditorError from '../../utils/ckeditorerror.js';
 import EmitterMixin from '../../utils/emittermixin.js';
 import { keyNames } from '../../utils/keyboard.js';
@@ -76,31 +76,39 @@ export default class Renderer {
 		 */
 		this._inlineFillerPosition = null;
 
-		this._domSelectionWindow = null;
+		this._domSelection = null;
 
 		this._listener = Object.create( EmitterMixin );
 
-		this._listener.listenTo( treeView, 'keydown', ( data ) => {
-			if ( data.keyCode != keyNames.arrowleft || !this._isInlineFillerAtSelection() ) {
+		this._listener.listenTo( treeView, 'keydown', ( evt, data ) => {
+			if ( data.keyCode != keyNames.arrowleft ) {
+				return;
+			}
+
+			if ( !this._isInlineFillerAtSelection() ) {
 				return;
 			}
 
 			const selectionPosition = this.selection.getFirstPosition();
 
-			if ( selectionPosition.offset != INLINE_FILLER_SIZE ) {
+			if ( selectionPosition.parent instanceof ViewText && selectionPosition.offset > 0 ) {
 				return;
 			}
 
-			const domParent = this.domConverter( selectionPosition.parent );
+			const selectionObserver = treeView.getObserver( SelectionObserver );
 
+			selectionObserver.disable();
 			// Damn iframe! I can not use global window, so element -> document -> window -> selection
-			const domSelection = domParent.ownerDocument.defaultView.getSelection();
+			const domSelection = window.getSelection();
+			const domParent = domSelection.getRangeAt( 0 ).startContainer;
 
 			const domRange = new Range();
-			domRange.setStart( domParent.parent, 0 );
+			domRange.setStart( domParent, 0 );
 			domRange.collapse( true );
 			domSelection.removeAllRanges();
 			domSelection.addRange( domRange );
+
+			selectionObserver.enable();
 		} );
 	}
 
@@ -169,6 +177,7 @@ export default class Renderer {
 
 			if ( this._needAddInlineFiller() ) {
 				this._inlineFillerPosition = selection.getFirstPosition();
+				this.markedChildren.add( this._inlineFillerPosition.parent );
 			} else {
 				this._inlineFillerPosition = null;
 			}
@@ -196,7 +205,7 @@ export default class Renderer {
 	}
 
 	_isInlineFillerAtSelection() {
-		if ( this.selection.rangeCount != 1 || !this.selection.isCollapsed() ) {
+		if ( this.selection.rangeCount != 1 || !this.selection.isCollapsed ) {
 			return false;
 		}
 
@@ -226,10 +235,10 @@ export default class Renderer {
 			return;
 		}
 
-		const domPosition = this.domConverter.viewPositionToDom( this._inlineFillerPosition );
-		const domText = domPosition.parent.childNodes[ domPosition.offset ];
+		const domFillerPosition = this.domConverter.viewPositionToDom( this._inlineFillerPosition );
+		const domFillerNode = domFillerPosition.parent;
 
-		if ( !this.domConverter.startsWithFiller( domText ) ) {
+		if ( !this.domConverter.startsWithFiller( domFillerNode ) ) {
 			/**
 			 * No inline filler on expected position.
 			 *
@@ -238,15 +247,15 @@ export default class Renderer {
 			throw new CKEditorError( 'renderer-render-no-inline-filler: No inline filler on expected position.' );
 		}
 
-		if ( this.domConverter.isInlineFiller( domText ) ) {
-			domPosition.parent.removeChild( domText );
+		if ( this.domConverter.isInlineFiller( domFillerNode ) ) {
+			domFillerNode.parentNode.removeChild( domFillerNode );
 		} else {
-			domText.data = domText.data.substr( INLINE_FILLER_SIZE );
+			domFillerNode.data = domFillerNode.data.substr( INLINE_FILLER_SIZE );
 		}
 	}
 
 	_needAddInlineFiller() {
-		if ( this.selection.rangeCount != 1 || !this.selection.isCollapsed() ) {
+		if ( this.selection.rangeCount != 1 || !this.selection.isCollapsed ) {
 			return false;
 		}
 
@@ -263,8 +272,8 @@ export default class Renderer {
 			return false;
 		}
 
-		const nodeBefore = selectionPosition.nodeBefore();
-		const nodeAfter = selectionPosition.nodeAfter();
+		const nodeBefore = selectionPosition.nodeBefore;
+		const nodeAfter = selectionPosition.nodeAfter;
 
 		if ( nodeBefore instanceof ViewText || nodeAfter instanceof ViewText ) {
 			return false;
@@ -357,10 +366,14 @@ export default class Renderer {
 	}
 
 	_updateSelection() {
-		const domSelection = this._domSelectionWindow && this._domSelectionWindow.getSelection();
+		let domSelection = window.getSelection();
 		const oldViewSelection = domSelection && this.domConverter.domSelectionToView( domSelection );
 
-		if ( ( !oldViewSelection && !this.selection.rangeCount ) || this.selection.isEqual( oldViewSelection ) ) {
+		if ( ( !oldViewSelection && !this.selection.rangeCount ) ) {
+			return;
+		}
+
+		if ( oldViewSelection && this.selection.isEqual( oldViewSelection ) ) {
 			return;
 		}
 
@@ -375,14 +388,13 @@ export default class Renderer {
 			const domRange = new Range();
 			domRange.setStart( domRangeStart.parent, domRangeStart.offset );
 			domRange.setEnd( domRangeEnd.parent, domRangeEnd.offset );
-			domSelection.addRange( range );
+			domRange.startContainer.ownerDocument.defaultView.getSelection().addRange( domRange );
 		}
 
 		if ( this.selection.rangeCount ) {
-			// Get window for selection: Selection -> Range -> element -> document -> window.
-			this._domSelectionWindow = domSelection.getRangeAt( 0 ).startContainer.ownerDocument.defaultView;
+			this._domSelection = domSelection;
 		} else {
-			this._domSelectionWindow = null;
+			this._domSelection = null;
 		}
 	}
 }

+ 11 - 1
packages/ckeditor5-engine/src/treeview/selection.js

@@ -215,7 +215,7 @@ export default class Selection {
 	isEqual( otherSelection ) {
 		const rangeCount = this.rangeCount;
 
-		if ( rangeCount != otherSelection.rangeCount() ) {
+		if ( rangeCount != otherSelection.rangeCount ) {
 			return false;
 		}
 
@@ -262,6 +262,16 @@ export default class Selection {
 		this.fire( 'change' );
 	}
 
+	setTo( otherSelection ) {
+		this.removeAllRanges();
+
+		for ( let range of otherSelection.getRanges() ) {
+			this._pushRange( range );
+		}
+
+		this._lastRangeBackward = otherSelection._lastRangeBackward;
+	}
+
 	/**
 	 * Collapses selection to the {@link engine.treeView.Selection#getFirstPosition first position} in stored ranges.
 	 * All ranges will be removed beside one collapsed range. Nothing will be changed if there are no ranges stored