8
0
Просмотр исходного кода

Changed: renamed `DomConverter.isCorrectDomSelection(Position)` to `DomConverter.isDomSelection(Position)Correct`.
Changed: moved `jumpOverUiElement` handler to `uielement.js`.
Docs: minor fixes.

Szymon Cofalik 8 лет назад
Родитель
Сommit
970908b413

+ 2 - 29
packages/ckeditor5-engine/src/view/document.js

@@ -12,6 +12,7 @@ import Renderer from './renderer';
 import DomConverter from './domconverter';
 import DomConverter from './domconverter';
 import RootEditableElement from './rooteditableelement';
 import RootEditableElement from './rooteditableelement';
 import { injectQuirksHandling } from './filler';
 import { injectQuirksHandling } from './filler';
+import { injectUiElementHandling } from './uielement';
 import log from '@ckeditor/ckeditor5-utils/src/log';
 import log from '@ckeditor/ckeditor5-utils/src/log';
 import MutationObserver from './observer/mutationobserver';
 import MutationObserver from './observer/mutationobserver';
 import SelectionObserver from './observer/selectionobserver';
 import SelectionObserver from './observer/selectionobserver';
@@ -20,7 +21,6 @@ import KeyObserver from './observer/keyobserver';
 import FakeSelectionObserver from './observer/fakeselectionobserver';
 import FakeSelectionObserver from './observer/fakeselectionobserver';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
-import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 
 
 /**
 /**
  * Document class creates an abstract layer over the content editable area.
  * Document class creates an abstract layer over the content editable area.
@@ -127,8 +127,7 @@ export default class Document {
 		this.addObserver( FakeSelectionObserver );
 		this.addObserver( FakeSelectionObserver );
 
 
 		injectQuirksHandling( this );
 		injectQuirksHandling( this );
-
-		this.on( 'keydown', ( evt, data ) => _jumpOverUiElement( evt, data, this.domConverter ) );
+		injectUiElementHandling( this );
 
 
 		this.decorate( 'render' );
 		this.decorate( 'render' );
 	}
 	}
@@ -350,29 +349,3 @@ mix( Document, ObservableMixin );
  *
  *
  * @event render
  * @event render
  */
  */
-
-// Selection cannot be placed in a `UIElement`. Whenever it is placed there, it is moved before it. This
-// causes a situation when it is impossible to jump over `UIElement` using right arrow key, because the selection
-// ends up in ui element (in DOM) and is moved back to the left. This handler fixes this situation.
-function _jumpOverUiElement( evt, data, domConverter ) {
-	if ( data.keyCode == keyCodes.arrowright ) {
-		const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();
-
-		if ( domSelection.rangeCount == 1 && domSelection.getRangeAt( 0 ).collapsed ) {
-			const domParent = domSelection.getRangeAt( 0 ).startContainer;
-			const domOffset = domSelection.getRangeAt( 0 ).startOffset;
-
-			const viewPosition = domConverter.domPositionToView( domParent, domOffset );
-			// Skip all following ui elements.
-			const nextViewPosition = viewPosition.getLastMatchingPosition( value => value.item.is( 'uiElement' ) );
-
-			// If anything has been skipped, fix position.
-			// This `if` could be possibly omitted but maybe it is better not to mess with DOM selection if not needed.
-			if ( !viewPosition.isEqual( nextViewPosition ) ) {
-				const newDomPosition = domConverter.viewPositionToDom( nextViewPosition );
-
-				domSelection.collapse( newDomPosition.parent, newDomPosition.offset );
-			}
-		}
-	}
-}

+ 15 - 13
packages/ckeditor5-engine/src/view/domconverter.js

@@ -839,34 +839,36 @@ export default class DomConverter {
 	}
 	}
 
 
 	/**
 	/**
-	 * Checks if given {Selection DOM Selection} boundaries are in correct places.
+	 * Checks if given selection's boundaries are at correct places.
 	 *
 	 *
-	 * Incorrect places for selection are:
-	 * * before or in the middle of inline filler sequence,
-	 * * inside DOM element that represents {@link module:engine/view/uielement~UIElement view ui element}.
+	 * The following places are considered as incorrect for selection boundaries:
+	 * * before or in the middle of the inline filler sequence,
+	 * * inside the DOM element which represents {@link module:engine/view/uielement~UIElement a view ui element}.
 	 *
 	 *
 	 * @param {Selection} domSelection DOM Selection object to be checked.
 	 * @param {Selection} domSelection DOM Selection object to be checked.
-	 * @returns {Boolean} `true` if given selection is at correct place, `false` otherwise.
+	 * @returns {Boolean} `true` if the given selection is at a correct place, `false` otherwise.
 	 */
 	 */
-	isCorrectDomSelection( domSelection ) {
-		return this._isCorrectDomSelectionPosition( domSelection.anchorNode, domSelection.anchorOffset ) &&
-			this._isCorrectDomSelectionPosition( domSelection.focusNode, domSelection.focusOffset );
+	isDomSelectionCorrect( domSelection ) {
+		return this._isDomSelectionPositionCorrect( domSelection.anchorNode, domSelection.anchorOffset ) &&
+			this._isDomSelectionPositionCorrect( domSelection.focusNode, domSelection.focusOffset );
 	}
 	}
 
 
 	/**
 	/**
-	 * Checks if given DOM position is a correct place for selection boundary. See {@link ~isCorrectDomSelection}.
+	 * Checks if the given DOM position is a correct place for selection boundary. See {@link ~isDomSelectionCorrect}.
 	 *
 	 *
 	 * @private
 	 * @private
-	 * @param {Node} domParent Position parent.
+	 * @param {Element} domParent Position parent.
 	 * @param {Number} offset Position offset.
 	 * @param {Number} offset Position offset.
-	 * @returns {Boolean} `true` if given position is correct place for selection boundary, `false` otherwise.
+	 * @returns {Boolean} `true` if given position is at a correct place for selection boundary, `false` otherwise.
 	 */
 	 */
-	_isCorrectDomSelectionPosition( domParent, offset ) {
+	_isDomSelectionPositionCorrect( domParent, offset ) {
 		// If selection is before or in the middle of inline filler string, it is incorrect.
 		// If selection is before or in the middle of inline filler string, it is incorrect.
 		if ( this.isText( domParent ) && startsWithFiller( domParent ) && offset < INLINE_FILLER_LENGTH ) {
 		if ( this.isText( domParent ) && startsWithFiller( domParent ) && offset < INLINE_FILLER_LENGTH ) {
 			// Selection in a text node, at wrong position (before or in the middle of filler).
 			// Selection in a text node, at wrong position (before or in the middle of filler).
 			return false;
 			return false;
-		} else if ( this.isElement( domParent ) && startsWithFiller( domParent.childNodes[ offset ] ) ) {
+		}
+
+		if ( this.isElement( domParent ) && startsWithFiller( domParent.childNodes[ offset ] ) ) {
 			// Selection in an element node, before filler text node.
 			// Selection in an element node, before filler text node.
 			return false;
 			return false;
 		}
 		}

+ 1 - 1
packages/ckeditor5-engine/src/view/observer/selectionobserver.js

@@ -148,7 +148,7 @@ export default class SelectionObserver extends Observer {
 		const domSelection = domDocument.defaultView.getSelection();
 		const domSelection = domDocument.defaultView.getSelection();
 		const newViewSelection = this.domConverter.domSelectionToView( domSelection );
 		const newViewSelection = this.domConverter.domSelectionToView( domSelection );
 
 
-		if ( this.selection.isEqual( newViewSelection ) && this.domConverter.isCorrectDomSelection( domSelection ) ) {
+		if ( this.selection.isEqual( newViewSelection ) && this.domConverter.isDomSelectionCorrect( domSelection ) ) {
 			return;
 			return;
 		}
 		}
 
 

+ 4 - 4
packages/ckeditor5-engine/src/view/renderer.js

@@ -603,12 +603,12 @@ export default class Renderer {
 	_updateDomSelection( domRoot ) {
 	_updateDomSelection( domRoot ) {
 		const domSelection = domRoot.ownerDocument.defaultView.getSelection();
 		const domSelection = domRoot.ownerDocument.defaultView.getSelection();
 
 
-		// Below we will check whether DOM Selection needs updating at all.
-		// We need to update DOM Selection if either:
+		// Below we will check whether DOM selection needs updating at all.
+		// We need to update DOM selection if either:
 		// * it is at incorrect position, or
 		// * it is at incorrect position, or
 		// * it has changed (when compared to view selection).
 		// * it has changed (when compared to view selection).
-		if ( this.domConverter.isCorrectDomSelection( domSelection ) ) {
-			// DOM Selection is at correct position. Check whether it has changed.
+		if ( this.domConverter.isDomSelectionCorrect( domSelection ) ) {
+			// DOM selection is at correct position. Check whether it has changed.
 			const viewSelectionFromDom = this.domConverter.domSelectionToView( domSelection );
 			const viewSelectionFromDom = this.domConverter.domSelectionToView( domSelection );
 
 
 			// Compare view selection assumed from dom with current view selection.
 			// Compare view selection assumed from dom with current view selection.

+ 37 - 0
packages/ckeditor5-engine/src/view/uielement.js

@@ -10,6 +10,7 @@
 import Element from './element';
 import Element from './element';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import Node from './node';
 import Node from './node';
+import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 
 
 /**
 /**
  * UIElement class. It is used to represent UI not a content of the document.
  * UIElement class. It is used to represent UI not a content of the document.
@@ -82,9 +83,45 @@ export default class UIElement extends Element {
 	}
 	}
 }
 }
 
 
+/**
+ * Assign key observer which will move cursor over a ui element if right arrow key is pressed and selection is before
+ * ui element.
+ *
+ * @param {module:engine/view/document~Document} document Document instance we should inject quirks handling on.
+ */
+export function injectUiElementHandling( document ) {
+	document.on( 'keydown', ( evt, data ) => jumpOverUiElement( evt, data, document.domConverter ) );
+}
+
 // Returns `null` because block filler is not needed for UIElements.
 // Returns `null` because block filler is not needed for UIElements.
 //
 //
 // @returns {null}
 // @returns {null}
 function getFillerOffset() {
 function getFillerOffset() {
 	return null;
 	return null;
 }
 }
+
+// Selection cannot be placed in a `UIElement`. Whenever it is placed there, it is moved before it. This
+// causes a situation when it is impossible to jump over `UIElement` using right arrow key, because the selection
+// ends up in ui element (in DOM) and is moved back to the left. This handler fixes this situation.
+function jumpOverUiElement( evt, data, domConverter ) {
+	if ( data.keyCode == keyCodes.arrowright ) {
+		const domSelection = data.domTarget.ownerDocument.defaultView.getSelection();
+
+		if ( domSelection.rangeCount == 1 && domSelection.getRangeAt( 0 ).collapsed ) {
+			const domParent = domSelection.getRangeAt( 0 ).startContainer;
+			const domOffset = domSelection.getRangeAt( 0 ).startOffset;
+
+			const viewPosition = domConverter.domPositionToView( domParent, domOffset );
+			// Skip all following ui elements.
+			const nextViewPosition = viewPosition.getLastMatchingPosition( value => value.item.is( 'uiElement' ) );
+
+			// If anything has been skipped, fix position.
+			// This `if` could be possibly omitted but maybe it is better not to mess with DOM selection if not needed.
+			if ( !viewPosition.isEqual( nextViewPosition ) ) {
+				const newDomPosition = domConverter.viewPositionToDom( nextViewPosition );
+
+				domSelection.collapse( newDomPosition.parent, newDomPosition.offset );
+			}
+		}
+	}
+}

+ 2 - 1
packages/ckeditor5-engine/tests/conversion/view-selection-to-model-converters.js

@@ -110,7 +110,8 @@ describe( 'convertSelectionChange', () => {
 	} );
 	} );
 
 
 	it( 'should re-convert selection even if it has not changed in model', () => {
 	it( 'should re-convert selection even if it has not changed in model', () => {
-		// Selection might not have changed in model but it needs to be reconverted because it ended up in incorrect place in DOM.
+		// Selection might have not changed in the model but it needs to be reconverted because
+		// it ended up at an incorrect place in the DOM.
 		const viewSelection = new ViewSelection();
 		const viewSelection = new ViewSelection();
 		viewSelection.addRange( ViewRange.createFromParentsAndOffsets(
 		viewSelection.addRange( ViewRange.createFromParentsAndOffsets(
 			viewRoot.getChild( 0 ).getChild( 0 ), 1, viewRoot.getChild( 0 ).getChild( 0 ), 1 ) );
 			viewRoot.getChild( 0 ).getChild( 0 ), 1, viewRoot.getChild( 0 ).getChild( 0 ), 1 ) );

+ 12 - 12
packages/ckeditor5-engine/tests/view/domconverter/domconverter.js

@@ -192,7 +192,7 @@ describe( 'DomConverter', () => {
 		} );
 		} );
 	} );
 	} );
 
 
-	describe( 'isCorrectDomSelection', () => {
+	describe( 'isDomSelectionCorrect()', () => {
 		function domSelection( anchorParent, anchorOffset, focusParent, focusOffset ) {
 		function domSelection( anchorParent, anchorOffset, focusParent, focusOffset ) {
 			const sel = document.getSelection();
 			const sel = document.getSelection();
 
 
@@ -223,15 +223,15 @@ describe( 'DomConverter', () => {
 		it( 'should return true for correct dom selection', () => {
 		it( 'should return true for correct dom selection', () => {
 			// <p>INLINE_FILLER{foo}<span></span></p>.
 			// <p>INLINE_FILLER{foo}<span></span></p>.
 			const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
 			const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
-			expect( converter.isCorrectDomSelection( sel1 ) ).to.be.true;
+			expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.true;
 
 
 			// <p>INLINE_FILLERfoo[]<span></span></p>.
 			// <p>INLINE_FILLERfoo[]<span></span></p>.
 			const sel2 = domSelection( domP, 1, domP, 1 );
 			const sel2 = domSelection( domP, 1, domP, 1 );
-			expect( converter.isCorrectDomSelection( sel2 ) ).to.be.true;
+			expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.true;
 
 
 			// <p>INLINE_FILLERfoo<span></span>[]</p>.
 			// <p>INLINE_FILLERfoo<span></span>[]</p>.
 			const sel3 = domSelection( domP, 2, domP, 2 );
 			const sel3 = domSelection( domP, 2, domP, 2 );
-			expect( converter.isCorrectDomSelection( sel3 ) ).to.be.true;
+			expect( converter.isDomSelectionCorrect( sel3 ) ).to.be.true;
 		} );
 		} );
 
 
 		describe( 'should return false', () => {
 		describe( 'should return false', () => {
@@ -239,40 +239,40 @@ describe( 'DomConverter', () => {
 				// Tests forward and backward selection.
 				// Tests forward and backward selection.
 				// <p>[INLINE_FILLERfoo]<span></span></p>.
 				// <p>[INLINE_FILLERfoo]<span></span></p>.
 				const sel1 = domSelection( domP, 0, domP, 1 );
 				const sel1 = domSelection( domP, 0, domP, 1 );
-				expect( converter.isCorrectDomSelection( sel1 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
 
 
 				const sel2 = domSelection( domP, 1, domP, 0 );
 				const sel2 = domSelection( domP, 1, domP, 0 );
-				expect( converter.isCorrectDomSelection( sel2 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
 			} );
 			} );
 
 
 			it( 'if anchor or focus is before filler sequence', () => {
 			it( 'if anchor or focus is before filler sequence', () => {
 				// Tests forward and backward selection.
 				// Tests forward and backward selection.
 				// <p>{INLINE_FILLERfoo}<span></span></p>.
 				// <p>{INLINE_FILLERfoo}<span></span></p>.
 				const sel1 = domSelection( domFillerTextNode, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
 				const sel1 = domSelection( domFillerTextNode, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
-				expect( converter.isCorrectDomSelection( sel1 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
 
 
 				const sel2 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domFillerTextNode, 0 );
 				const sel2 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domFillerTextNode, 0 );
-				expect( converter.isCorrectDomSelection( sel2 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
 			} );
 			} );
 
 
 			it( 'if anchor or focus is in the middle of filler sequence', () => {
 			it( 'if anchor or focus is in the middle of filler sequence', () => {
 				// Tests forward and backward selection.
 				// Tests forward and backward selection.
 				// <p>I{NLINE_FILLERfoo}<span></span></p>.
 				// <p>I{NLINE_FILLERfoo}<span></span></p>.
 				const sel1 = domSelection( domFillerTextNode, 1, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
 				const sel1 = domSelection( domFillerTextNode, 1, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
-				expect( converter.isCorrectDomSelection( sel1 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
 
 
 				const sel2 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domFillerTextNode, 1 );
 				const sel2 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domFillerTextNode, 1 );
-				expect( converter.isCorrectDomSelection( sel2 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
 			} );
 			} );
 
 
 			it( 'if anchor or focus is inside dom element that represents view ui element', () => {
 			it( 'if anchor or focus is inside dom element that represents view ui element', () => {
 				// Tests forward and backward selection.
 				// Tests forward and backward selection.
 				// <p>INLINE_FILLER{foo<span>]</span></p>.
 				// <p>INLINE_FILLER{foo<span>]</span></p>.
 				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domUiSpan, 0 );
 				const sel1 = domSelection( domFillerTextNode, INLINE_FILLER_LENGTH + 3, domUiSpan, 0 );
-				expect( converter.isCorrectDomSelection( sel1 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel1 ) ).to.be.false;
 
 
 				const sel2 = domSelection( domUiSpan, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
 				const sel2 = domSelection( domUiSpan, 0, domFillerTextNode, INLINE_FILLER_LENGTH + 3 );
-				expect( converter.isCorrectDomSelection( sel2 ) ).to.be.false;
+				expect( converter.isDomSelectionCorrect( sel2 ) ).to.be.false;
 			} );
 			} );
 		} );
 		} );
 	} );
 	} );