Additional tests + fixes after the review.

ckeditor · Jan 25, 2021 · 37e74ec · 37e74ec
1 parent 51408da
commit 37e74ec
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 5 deletions.
diff --git a/packages/ckeditor5-media-embed/src/mediaembedcommand.js b/packages/ckeditor5-media-embed/src/mediaembedcommand.js
@@ -88,5 +88,5 @@ function isAllowedInParent( selection, model ) {
 // @returns {Boolean}
 function isMediaSelected( selection ) {
 	const element = selection.getSelectedElement();
-	return element && element.name === 'media' || false;
+	return !!element && element.name === 'media';
 }
diff --git a/packages/ckeditor5-widget/src/utils.js b/packages/ckeditor5-widget/src/utils.js
@@ -305,7 +305,7 @@ export function findOptimalInsertionPosition( selection, model ) {
 }
 
 /**
- * Checks if the selection is on object.
+ * Checks if the selection is on an object.
  *
  * @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
  * @param {module:engine/model/schema~Schema} schema
@@ -314,7 +314,7 @@ export function findOptimalInsertionPosition( selection, model ) {
 export function checkSelectionOnObject( selection, schema ) {
 	const selectedElement = selection.getSelectedElement();
 
-	return selectedElement && schema.isObject( selectedElement ) || false;
+	return !!selectedElement && schema.isObject( selectedElement );
 }
 
 /**

diff --git a/packages/ckeditor5-widget/tests/utils.js b/packages/ckeditor5-widget/tests/utils.js
@@ -525,6 +525,10 @@ describe( 'widget utils', () => {
 				allowIn: '$root',
 				isSelectable: true
 			} );
+
+			model.schema.extend( '$text', {
+				allowIn: 'image'
+			} );
 		} );
 
 		it( 'should return false if no element is selected', () => {
@@ -536,7 +540,7 @@ describe( 'widget utils', () => {
 			expect( isSelectionOnObject ).to.be.false;
 		} );
 
-		it( 'should return false if the selection is not on the object', () => {
+		it( 'should return false if the selection is not on an object', () => {
 			setData( model, '[<element></element>]' );
 
 			const selection = model.document.selection;
@@ -545,14 +549,32 @@ describe( 'widget utils', () => {
 			expect( isSelectionOnObject ).to.be.false;
 		} );
 
-		it( 'should return true if the selection is on the object', () => {
+		it( 'should return true if the selection is on an object', () => {
 			setData( model, '<paragraph></paragraph>[<image></image>]' );
 
 			const selection = model.document.selection;
 			const isSelectionOnObject = checkSelectionOnObject( selection, model.schema );
 
 			expect( isSelectionOnObject ).to.be.true;
 		} );
+
+		it( 'should return false if the selection contains an object', () => {
+			setData( model, '<paragraph>fo[o</paragraph><image></image><paragraph>ba]r</paragraph>' );
+
+			const selection = model.document.selection;
+			const isSelectionOnObject = checkSelectionOnObject( selection, model.schema );
+
+			expect( isSelectionOnObject ).to.be.false;
+		} );
+
+		it( 'should return false if the selection is nested in an object', () => {
+			setData( model, '<image>[foo]</image>' );
+
+			const selection = model.document.selection;
+			const isSelectionOnObject = checkSelectionOnObject( selection, model.schema );
+
+			expect( isSelectionOnObject ).to.be.false;
+		} );
 	} );
 
 	describe( 'viewToModelPositionOutsideModelElement()', () => {