Merge pull request #1104 from ckeditor/t/1027

Widget has now correct selection behaviour inside tables

CHANGES.md

@@ -34,6 +34,7 @@ Fixed Issues:
 * [#1213](https://github.com/ckeditor/ckeditor-dev/issues/1213): Fixed: Multiple images uploaded using [Upload Image](https://ckeditor.com/cke4/addon/uploadimage) plugin are randomly duplicated or mangled.
 * [#532](https://github.com/ckeditor/ckeditor-dev/issues/532): Fixed: Removed an outdated user guide link from the [About](https://ckeditor.com/cke4/addon/about) dialog.
 * [#1221](https://github.com/ckeditor/ckeditor-dev/issues/1221): Fixed: Invalid CSS loaded by [Balloon Panel](https://ckeditor.com/cke4/addon/balloonpanel) plugin when [`config.skin`](https://docs.ckeditor.com/ckeditor4/docs/#!/api/CKEDITOR.config-cfg-skin) is loaded using a custom path.
+* [#522](https://github.com/ckeditor/ckeditor-dev/issues/522): Fixed: Widget selection is not removed when widget is inside table cell with [Table Selection](https://ckeditor.com/cke4/addon/tableselection) plugin enabled. Fixes also [#1027](https://github.com/ckeditor/ckeditor-dev/issues/1027).
 
 API Changes:
 

core/selection.js

@@ -11,6 +11,10 @@
 		fillingCharSequenceRegExp = new RegExp( fillingCharSequence + '( )?', 'g' ),
 		isSelectingTable;
 
+	function isWidget( element ) {
+		return CKEDITOR.plugins.widget && CKEDITOR.plugins.widget.isDomWidget( element );
+	}
+
 	// #### table selection : START
 	// @param {CKEDITOR.dom.range[]} ranges
 	// @param {Boolean} allowPartially Whether a collapsed selection within table is recognized to be a valid selection.
@@ -19,7 +23,10 @@
 		if ( ranges.length === 0 ) {
 			return false;
 		}
-
+		// It's not table selection when selected node is a widget (#1027).
+		if ( isWidget( ranges[ 0 ].getEnclosedNode() ) ) {
+			return false;
+		}
 		var node,
 			i;
 
@@ -96,6 +103,11 @@
 			return table.equals( fakeTable ) || fakeTable.contains( table );
 		}
 
+		// When widget is selected, then definitely is not a table (#1027).
+		if ( isWidget( fakeSelection.getSelectedElement() ) ) {
+			return false;
+		}
+
 		if ( isValidTableSelection( table, fakeTable, ranges, fakeRanges ) ) {
 			// Edge case: when editor contains only table and that table is selected using selectAll command,
 			// then the selection is not properly refreshed and it must be done manually.
@@ -1907,7 +1919,6 @@
 				// faked or its clone.
 				if ( this.rev == editor._.fakeSelection.rev ) {
 					delete editor._.fakeSelection;
-
 					removeHiddenSelectionContainer( editor );
 				}
 				else {

plugins/tableselection/plugin.js

@@ -16,6 +16,10 @@
 		insertRow,
 		insertColumn;
 
+	function isWidget( element ) {
+		return CKEDITOR.plugins.widget && CKEDITOR.plugins.widget.isDomWidget( element );
+	}
+
 	function getCellsBetween( first, last ) {
 		var firstTable = first.getAscendant( 'table' ),
 			lastTable = last.getAscendant( 'table' ),
@@ -208,7 +212,8 @@
 			// The selection is inside one cell, so we should allow native selection,
 			// but only in case if no other cell between mousedown and mouseup
 			// was selected.
-			if ( !fakeSelection.dirty && cells.length === 1 ) {
+			// We don't want to clear selection if widget is event target (#1027).
+			if ( !fakeSelection.dirty && cells.length === 1 && !( isWidget( evt.data.getTarget() ) ) ) {
 				return clearFakeCellSelection( editor, evt.name === 'mouseup' );
 			}
 
@@ -269,15 +274,19 @@
 		if ( !evt.data.getTarget().getName ) {
 			return;
 		}
+		// Prevent of applying table selection when widget is selected.
+		// Mouseup remain possibility to finish table selection when user release mouse button above widget in table.
+		if ( evt.name !== 'mouseup' && isWidget( evt.data.getTarget() ) ) {
+			return;
+		}
 
 		var editor = evt.editor || evt.listenerData.editor,
 			selection = editor.getSelection( 1 ),
 			selectedTable = getFakeSelectedTable( editor ),
 			target = evt.data.getTarget(),
 			cell = target && target.getAscendant( { td: 1, th: 1 }, true ),
 			table = target && target.getAscendant( 'table', true ),
-			tableElements = { table: 1, thead: 1, tbody: 1, tfoot: 1, tr: 1, td: 1, th: 1 },
-			canClear;
+			tableElements = { table: 1, thead: 1, tbody: 1, tfoot: 1, tr: 1, td: 1, th: 1 };
 
 		// Nested tables should be treated as the same one (e.g. user starts dragging from outer table
 		// and ends in inner one).
@@ -310,7 +319,7 @@
 			return false;
 		}
 
-		if ( canClear = canClearSelection( evt, selection, selectedTable, table ) ) {
+		if ( canClearSelection( evt, selection, selectedTable, table ) ) {
 			clearFakeCellSelection( editor, true );
 		}
 

plugins/widget/plugin.js

@@ -1584,6 +1584,18 @@
 		return node.type == CKEDITOR.NODE_ELEMENT && node.hasAttribute( 'data-cke-widget-wrapper' );
 	};
 
+	/**
+	 * Checks whether the `node` is a DOM widget.
+	 *
+	 * @since 4.8.0
+	 * @static
+	 * @param {CKEDITOR.dom.node} node
+	 * @returns {Boolean}
+	 */
+	Widget.isDomWidget = function( node ) {
+		return node ? this.isDomWidgetWrapper( node ) || this.isDomWidgetElement( node ) : false;
+	};
+
 	/**
 	 * Checks whether the `node` is a {@link CKEDITOR.plugins.widget#element widget element}.
 	 *

tests/plugins/tableselection/integrations/image2/manual/deselectwidget.html

@@ -0,0 +1,34 @@
+<label>Lena's path: <input style="border:2px solid black" type="text" value="%BASE_PATH%_assets/lena.jpg" readonly></label>
+<textarea id="editor">
+	<table border="1" cellpadding="1" cellspacing="1" style="width:500px">
+		<tbody>
+			<tr>
+				<td>Text</td>
+				<td>Test</td>
+			</tr>
+			<tr>
+				<td><img alt="Lena" src="%BASE_PATH%_assets/lena.jpg" style="height:150px; width:150px" /></td>
+				<td>&nbsp;</td>
+			</tr>
+			<tr>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+			</tr>
+		</tbody>
+	</table>
+
+	<p>&nbsp;</p>
+
+</textarea>
+<script>
+	CKEDITOR.replace( 'editor', {
+		extraPlugins: 'image2',
+		on: {
+			pluginsLoaded: function() {
+				if ( !CKEDITOR.plugins.tableselection.isSupportedEnvironment ) {
+					bender.ignore();
+				}
+			}
+		}
+	} );
+</script>

tests/plugins/tableselection/integrations/image2/manual/deselectwidget.md

@@ -0,0 +1,15 @@
+@bender-tags: 1027, 4.8.0, widget, bug
+@bender-ui: collapsed
+@bender-ckeditor-plugins: undo,clipboard,basicstyles,toolbar,wysiwygarea,widget,table,tableselection,resize,image2
+
+1. Click on the image of Lena.
+1. Click into another cell and start to: _(Do not click outside of the table!)_
+  * type something
+  * add new image (path to lena's image is above editor)
+
+**Expected:**
+  * You can freely type and edit text.
+  * You can add new images.
+  * Image dosen't persist the selection.
+
+**Unexpected:** You can't do things listed in 'expected'. Selection remain on image all the time.

tests/plugins/widget/integration/table/_helpers/testwidgets.js

@@ -0,0 +1,29 @@
+CKEDITOR.plugins.add( 'inlineWidget', {
+	requires: 'widget',
+	init: function( editor ) {
+		editor.widgets.add( 'test1', {
+			button: 'Create Inline Widget',
+			template: '<strong class="inline-widget" style="background-color:#C0392B;padding:5px;">Very strong text.</strong>',
+			requiredContent: 'strong(inline-widget)',
+			allowedContent: 'strong(!inline-widget){*}',
+			upcast: function( element ) {
+				return element.name === 'strong' && element.hasClass( 'inline-widget' );
+			}
+		} );
+	}
+} );
+
+CKEDITOR.plugins.add( 'blockWidget', {
+	requires: 'widget',
+	init: function( editor ) {
+		editor.widgets.add( 'test2', {
+			button: 'Create Block Widget',
+			template: '<div class="block-widget" style="background-color:#F4D03F;padding:5px;">Some text in div.</div>',
+			requiredContent: 'div(block-widget)',
+			allowedContent: 'div(!block-widget){*}',
+			upcast: function( element ) {
+				return element.name === 'div' && element.hasClass( 'block-widget' );
+			}
+		} );
+	}
+} );

tests/plugins/widget/integration/table/manual/widgetintable.html

@@ -0,0 +1,77 @@
+<textarea id="classic" >
+	<table border="1" cellspacing="1" cellpadding="1" style="width:500px">
+		<tbody>
+			<tr>
+				<td><strong style="background-color:#C0392B; padding:5px" class="inline-widget">Very strong text.</strong></td>
+				<td>Some text</td>
+			</tr>
+			<tr>
+				<td></td>
+				<td></td>
+			</tr>
+			<tr>
+				<td>
+					<table border="1" cellspacing="1" cellpadding="1" style="width:200px">
+						<tbody>
+							<tr>
+								<td></td>
+								<td></td>
+							</tr>
+							<tr>
+								<td><div style="background-color:#F4D03F;padding:5px;" class="block-widget">Some text in div.</div><p> </p></td>
+								<td>Nested Table</td>
+							</tr>
+						</tbody>
+					</table>
+				</td>
+				<td>Some text</td>
+			</tr>
+		</tbody>
+	</table>
+	<p> </p>
+</textarea>
+
+<div id="divarea">
+	<table border="1" cellspacing="1" cellpadding="1" style="width:500px">
+		<tbody>
+			<tr>
+				<td></td>
+				<td></td>
+			</tr>
+			<tr>
+				<td><div style="background-color:#F4D03F;padding:5px;" class="block-widget">Some text in div.</div><p> </p></td>
+				<td>Some text</td>
+			</tr>
+			<tr>
+				<td>
+					<table border="1" cellspacing="1" cellpadding="1" style="width:200px">
+						<tbody>
+							<tr>
+								<td><strong style="background-color:#C0392B; padding:5px" class="inline-widget">Very strong text.</strong></td>
+								<td>Nested Table</td>
+							</tr>
+							<tr>
+								<td></td>
+								<td></td>
+							</tr>
+						</tbody>
+					</table>
+				</td>
+				<td>Some text</td>
+			</tr>
+		</tbody>
+	</table>
+	<p> </p>
+</div>
+
+<script>
+	CKEDITOR.replace( 'classic', {
+		extraPlugins: 'inlineWidget,blockWidget',
+		removePlugins: 'tableselection',
+	} );
+
+	CKEDITOR.replace( 'divarea', {
+		removePlugins: 'tableselection',
+		extraPlugins: 'divarea,inlineWidget,blockWidget'
+	} );
+</script>

tests/plugins/widget/integration/table/manual/widgetintable.md

@@ -0,0 +1,17 @@
+@bender-tags: 1027, 4.8.0, widget, bug
+@bender-ui: collapsed
+@bender-include: ../_helpers/testwidgets.js
+@bender-ckeditor-plugins: undo,clipboard,basicstyles,toolbar,wysiwygarea,widget,table,resize
+
+----
+
+1. Try to add new widget in table (buttons without icon in toolbar).
+1. Click on the widgets in table.
+1. Try to select some text **in table**.
+1. Perform some operation on selected text (change, append new text, apply some style, etc.).
+
+_Perform above steps for all widgets, inner and outer table, and both editors._
+
+**Expected:** Widget is deselect when focus is changed to text in table and text selection is visible. You are able to perform operations on selected text.
+
+**Unexpected:** Widget preserve focus even tough some text in table is selected. You cannot modify selected text in such case.

tests/plugins/widget/integration/table/manual/widgetintablewithtableselection.html

@@ -0,0 +1,82 @@
+<textarea id="classic" >
+	<table border="1" cellspacing="1" cellpadding="1" style="width:500px">
+		<tbody>
+			<tr>
+				<td><strong style="background-color:#C0392B; padding:5px" class="inline-widget">Very strong text.</strong></td>
+				<td>Some text</td>
+			</tr>
+			<tr>
+				<td></td>
+				<td></td>
+			</tr>
+			<tr>
+				<td>
+					<table border="1" cellspacing="1" cellpadding="1" style="width:200px">
+						<tbody>
+							<tr>
+								<td></td>
+								<td></td>
+							</tr>
+							<tr>
+								<td><div style="background-color:#F4D03F;padding:5px;" class="block-widget">Some text in div.</div><p> </p></td>
+								<td>Nested Table</td>
+							</tr>
+						</tbody>
+					</table>
+				</td>
+				<td>Some text</td>
+			</tr>
+		</tbody>
+	</table>
+	<p> </p>
+</textarea>
+
+<div id="divarea">
+	<table border="1" cellspacing="1" cellpadding="1" style="width:500px">
+		<tbody>
+			<tr>
+				<td></td>
+				<td></td>
+			</tr>
+			<tr>
+				<td><div style="background-color:#F4D03F;padding:5px;" class="block-widget">Some text in div.</div><p> </p></td>
+				<td>Some text</td>
+			</tr>
+			<tr>
+				<td>
+					<table border="1" cellspacing="1" cellpadding="1" style="width:200px">
+						<tbody>
+							<tr>
+								<td><strong style="background-color:#C0392B; padding:5px" class="inline-widget">Very strong text.</strong></td>
+								<td>Nested Table</td>
+							</tr>
+							<tr>
+								<td></td>
+								<td></td>
+							</tr>
+						</tbody>
+					</table>
+				</td>
+				<td>Some text</td>
+			</tr>
+		</tbody>
+	</table>
+	<p> </p>
+</div>
+
+<script>
+	CKEDITOR.replace( 'classic', {
+		extraPlugins: 'inlineWidget,blockWidget',
+		on: {
+			pluginsLoaded: function() {
+				if ( !CKEDITOR.plugins.tableselection.isSupportedEnvironment ) {
+					bender.ignore();
+				}
+			}
+		}
+	} );
+
+	CKEDITOR.replace( 'divarea', {
+		extraPlugins: 'divarea,inlineWidget,blockWidget'
+	} );
+</script>

tests/plugins/widget/integration/table/manual/widgetintablewithtableselection.md

@@ -0,0 +1,17 @@
+@bender-tags: 1027, 4.8.0, widget, bug
+@bender-ui: collapsed
+@bender-include: ../_helpers/testwidgets.js
+@bender-ckeditor-plugins: undo,clipboard,basicstyles,toolbar,wysiwygarea,widget,table,resize,tableselection
+
+----
+
+1. Try to add new widget in table (buttons without icon in toolbar).
+1. Click on the widgets in table.
+1. Try to select some text **in table**.
+1. Perform some operation on selected text (change, append new text, apply some style, etc.).
+
+_Perform above steps for all widgets, inner and outer table, and both editors._
+
+**Expected:** Widget is deselect when focus is changed to text in table and text selection is visible. You are able to perform operations on selected text.
+
+**Unexpected:** Widget preserve focus even tough some text in table is selected. You cannot modify selected text in such case.