Merge pull request #8899 from ckeditor/i/8798-disable-commands-embed-…

…html

Fix (table): The `insertTable` command should be disabled if any object is selected. Closes #8798.

Fix (media-embed): The `insertMediaEmbed` command should be disabled if any non-media object is selected (see #8798).

Other (widget): The `checkSelectionOnObject` function should be exported by the `@ckeditor/ckeditor5-widget/src/utils package` (see #8798).

Tests (html-embed): The manual test should show the insert table and insert media buttons and describe their correct state (see #8798).

packages/ckeditor5-horizontal-line/src/horizontallinecommand.js

@@ -8,7 +8,7 @@
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
-import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils';
+import { findOptimalInsertionPosition, checkSelectionOnObject } from '@ckeditor/ckeditor5-widget/src/utils';
 
 /**
  * The horizontal line command.
@@ -86,17 +86,6 @@ function isHorizontalLineAllowedInParent( selection, schema, model ) {
 	return schema.checkChild( parent, 'horizontalLine' );
 }
 
-// Checks if the selection is on object.
-//
-// @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
-// @param {module:engine/model/schema~Schema} schema
-// @returns {Boolean}
-function checkSelectionOnObject( selection, schema ) {
-	const selectedElement = selection.getSelectedElement();
-
-	return selectedElement && schema.isObject( selectedElement );
-}
-
 // Returns a node that will be used to insert a horizontal line with `model.insertContent` to check if the horizontal line can be
 // placed there.
 //

packages/ckeditor5-html-embed/package.json

@@ -20,6 +20,8 @@
     "@ckeditor/ckeditor5-editor-classic": "^24.0.0",
     "@ckeditor/ckeditor5-engine": "^24.0.0",
     "@ckeditor/ckeditor5-paragraph": "^24.0.0",
+    "@ckeditor/ckeditor5-media-embed": "^24.0.0",
+    "@ckeditor/ckeditor5-table": "^24.0.0",
     "@ckeditor/ckeditor5-clipboard": "^24.0.0",
     "lodash-es": "^4.17.15",
     "sanitize-html": "^2.1.0"

packages/ckeditor5-html-embed/src/inserthtmlembedcommand.js

@@ -8,7 +8,7 @@
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
-import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils';
+import { findOptimalInsertionPosition, checkSelectionOnObject } from '@ckeditor/ckeditor5-widget/src/utils';
 
 /**
  * The insert HTML embed element command.
@@ -70,17 +70,6 @@ function isHtmlEmbedAllowedInParent( selection, schema, model ) {
 	return schema.checkChild( parent, 'rawHtml' );
 }
 
-// Checks if the selection is on object.
-//
-// @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
-// @param {module:engine/model/schema~Schema} schema
-// @returns {Boolean}
-function checkSelectionOnObject( selection, schema ) {
-	const selectedElement = selection.getSelectedElement();
-
-	return selectedElement && schema.isObject( selectedElement );
-}
-
 // Returns a node that will be used to insert a page break with `model.insertContent` to check if a html embed element can be placed there.
 //
 // @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection

packages/ckeditor5-html-embed/tests/manual/htmlembed.js

@@ -9,6 +9,8 @@ import sanitizeHtml from 'sanitize-html';
 import { clone } from 'lodash-es';
 import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
 import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
+import MediaEmbed from '@ckeditor/ckeditor5-media-embed/src/mediaembed';
+import Table from '@ckeditor/ckeditor5-table/src/table';
 import Code from '@ckeditor/ckeditor5-basic-styles/src/code';
 import HtmlEmbed from '../../src/htmlembed';
 
@@ -73,11 +75,11 @@ async function reloadEditor( config = {} ) {
 
 	config = {
 		...config,
-		plugins: [ ArticlePluginSet, HtmlEmbed, Code ],
+		plugins: [ ArticlePluginSet, HtmlEmbed, Code, MediaEmbed, Table ],
 		toolbar: [
 			'heading', '|', 'bold', 'italic', 'link', '|',
 			'bulletedList', 'numberedList', 'blockQuote', 'insertTable', '|',
-			'undo', 'redo', '|', 'htmlEmbed'
+			'undo', 'redo', '|', 'htmlEmbed', 'mediaEmbed'
 		],
 		image: {
 			toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ]

packages/ckeditor5-html-embed/tests/manual/htmlembed.md

@@ -41,3 +41,7 @@ elements or attributes may be not rendered in the editing view. However, they st
 You can disable the preview mode and have the `textarea` element that will be blocked or enabled (depends on feature state).
 
 For toggling the state (edit source / see preview), click the icon in the right-top corner.
+
+---
+
+If the HTML snippet is selected none of the commands for inserting an object should be enabled (media, html-embed, table).

packages/ckeditor5-image/src/image/utils.js

@@ -7,7 +7,7 @@
  * @module image/image/utils
  */
 
-import { findOptimalInsertionPosition, isWidget, toWidget } from '@ckeditor/ckeditor5-widget/src/utils';
+import { findOptimalInsertionPosition, checkSelectionOnObject, isWidget, toWidget } from '@ckeditor/ckeditor5-widget/src/utils';
 
 /**
  * Converts a given {@link module:engine/view/element~Element} to an image widget:
@@ -142,15 +142,6 @@ function isImageAllowedInParent( selection, schema, model ) {
 	return schema.checkChild( parent, 'image' );
 }
 
-// Check if selection is on object.
-//
-// @returns {Boolean}
-function checkSelectionOnObject( selection, schema ) {
-	const selectedElement = selection.getSelectedElement();
-
-	return selectedElement && schema.isObject( selectedElement );
-}
-
 // Checks if selection is placed in other image (ie. in caption).
 function isInOtherImage( selection ) {
 	return [ ...selection.focus.getAncestors() ].every( ancestor => !ancestor.is( 'element', 'image' ) );

packages/ckeditor5-media-embed/src/mediaembedcommand.js

@@ -8,7 +8,7 @@
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
-import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils';
+import { findOptimalInsertionPosition, checkSelectionOnObject } from '@ckeditor/ckeditor5-widget/src/utils';
 import { getSelectedMediaModelWidget, insertMedia } from './utils';
 
 /**
@@ -30,18 +30,13 @@ export default class MediaEmbedCommand extends Command {
 		const model = this.editor.model;
 		const selection = model.document.selection;
 		const schema = model.schema;
-		const insertPosition = findOptimalInsertionPosition( selection, model );
 		const selectedMedia = getSelectedMediaModelWidget( selection );
 
-		let parent = insertPosition.parent;
-
-		// The model.insertContent() will remove empty parent (unless it is a $root or a limit).
-		if ( parent.isEmpty && !model.schema.isLimit( parent ) ) {
-			parent = parent.parent;
-		}
-
 		this.value = selectedMedia ? selectedMedia.getAttribute( 'url' ) : null;
-		this.isEnabled = schema.checkChild( parent, 'media' );
+
+		this.isEnabled = isMediaSelected( selection ) ||
+			isAllowedInParent( selection, model ) &&
+			!checkSelectionOnObject( selection, schema );
 	}
 
 	/**
@@ -69,3 +64,29 @@ export default class MediaEmbedCommand extends Command {
 		}
 	}
 }
+
+// Checks if the table is allowed in the parent.
+//
+// @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
+// @param {module:engine/model/schema~Schema} schema
+// @returns {Boolean}
+function isAllowedInParent( selection, model ) {
+	const insertPosition = findOptimalInsertionPosition( selection, model );
+	let parent = insertPosition.parent;
+
+	// The model.insertContent() will remove empty parent (unless it is a $root or a limit).
+	if ( parent.isEmpty && !model.schema.isLimit( parent ) ) {
+		parent = parent.parent;
+	}
+
+	return model.schema.checkChild( parent, 'media' );
+}
+
+// Checks if the media object is selected.
+//
+// @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
+// @returns {Boolean}
+function isMediaSelected( selection ) {
+	const element = selection.getSelectedElement();
+	return !!element && element.name === 'media';
+}

packages/ckeditor5-media-embed/tests/insertmediacommand.js

@@ -88,6 +88,20 @@ describe( 'MediaEmbedCommand', () => {
 			setData( model, '<block><limit>foo[]</limit></block>' );
 			expect( command.isEnabled ).to.be.false;
 		} );
+
+		it( 'should be true if a non-object element is selected', () => {
+			model.schema.register( 'element', { allowIn: '$root', isSelectable: true } );
+
+			setData( model, '[<element></element>]' );
+			expect( command.isEnabled ).to.be.true;
+		} );
+
+		it( 'should be false if a non-media object is selected', () => {
+			model.schema.register( 'image', { isObject: true, isBlock: true, allowWhere: '$block' } );
+
+			setData( model, '[<image src="http://ckeditor.com"></image>]' );
+			expect( command.isEnabled ).to.be.false;
+		} );
 	} );
 
 	describe( 'value', () => {

packages/ckeditor5-page-break/src/pagebreakcommand.js

@@ -8,7 +8,7 @@
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
-import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils';
+import { findOptimalInsertionPosition, checkSelectionOnObject } from '@ckeditor/ckeditor5-widget/src/utils';
 
 /**
  * The page break command.
@@ -86,17 +86,6 @@ function isPageBreakAllowedInParent( selection, schema, model ) {
 	return schema.checkChild( parent, 'pageBreak' );
 }
 
-// Checks if the selection is on object.
-//
-// @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
-// @param {module:engine/model/schema~Schema} schema
-// @returns {Boolean}
-function checkSelectionOnObject( selection, schema ) {
-	const selectedElement = selection.getSelectedElement();
-
-	return selectedElement && schema.isObject( selectedElement );
-}
-
 // Returns a node that will be used to insert a page break with `model.insertContent` to check if the page break can be placed there.
 //
 // @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection

packages/ckeditor5-table/src/commands/inserttablecommand.js

@@ -8,7 +8,7 @@
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
-import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils';
+import { findOptimalInsertionPosition, checkSelectionOnObject } from '@ckeditor/ckeditor5-widget/src/utils';
 
 /**
  * The insert table command.
@@ -30,9 +30,8 @@ export default class InsertTableCommand extends Command {
 		const selection = model.document.selection;
 		const schema = model.schema;
 
-		const validParent = getInsertTableParent( selection.getFirstPosition() );
-
-		this.isEnabled = schema.checkChild( validParent, 'table' );
+		this.isEnabled = isAllowedInParent( selection, schema ) &&
+			!checkSelectionOnObject( selection, schema );
 	}
 
 	/**
@@ -64,11 +63,14 @@ export default class InsertTableCommand extends Command {
 	}
 }
 
-// Returns valid parent to insert table
+// Checks if the table is allowed in the parent.
 //
-// @param {module:engine/model/position} position
-function getInsertTableParent( position ) {
-	const parent = position.parent;
+// @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
+// @param {module:engine/model/schema~Schema} schema
+// @returns {Boolean}
+function isAllowedInParent( selection, schema ) {
+	const positionParent = selection.getFirstPosition().parent;
+	const validParent = positionParent === positionParent.root ? positionParent : positionParent.parent;
 
-	return parent === parent.root ? parent : parent.parent;
+	return schema.checkChild( validParent, 'table' );
 }

packages/ckeditor5-table/tests/commands/inserttablecommand.js

@@ -34,6 +34,11 @@ describe( 'InsertTableCommand', () => {
 
 	describe( 'isEnabled', () => {
 		describe( 'when selection is collapsed', () => {
+			it( 'should be true if in a root', () => {
+				setData( model, '[]' );
+				expect( command.isEnabled ).to.be.true;
+			} );
+
 			it( 'should be true if in paragraph', () => {
 				setData( model, '<paragraph>foo[]</paragraph>' );
 				expect( command.isEnabled ).to.be.true;
@@ -44,6 +49,27 @@ describe( 'InsertTableCommand', () => {
 				expect( command.isEnabled ).to.be.false;
 			} );
 		} );
+
+		describe( 'when selection is not collapsed', () => {
+			it( 'should be false if an object is selected', () => {
+				model.schema.register( 'media', { isObject: true, isBlock: true, allowWhere: '$block' } );
+
+				setData( model, '[<media url="http://ckeditor.com"></media>]' );
+				expect( command.isEnabled ).to.be.false;
+			} );
+
+			it( 'should be true if in a paragraph', () => {
+				setData( model, '<paragraph>[Foo]</paragraph>' );
+				expect( command.isEnabled ).to.be.true;
+			} );
+
+			it( 'should be true if a non-object element is selected', () => {
+				model.schema.register( 'element', { allowIn: '$root', isSelectable: true } );
+
+				setData( model, '[<element></element>]' );
+				expect( command.isEnabled ).to.be.true;
+			} );
+		} );
 	} );
 
 	describe( 'execute()', () => {

packages/ckeditor5-widget/src/utils.js

@@ -304,6 +304,19 @@ export function findOptimalInsertionPosition( selection, model ) {
 	return selection.focus;
 }
 
+/**
+ * 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
+ * @returns {Boolean}
+*/
+export function checkSelectionOnObject( selection, schema ) {
+	const selectedElement = selection.getSelectedElement();
+
+	return !!selectedElement && schema.isObject( selectedElement );
+}
+
 /**
  * A util to be used in order to map view positions to correct model positions when implementing a widget
  * which renders non-empty view element for an empty model element.