Merge pull request #3059 from ckeditor/t/2235

Fix for: Image in table cell has an empty URL field when edited from context menu opened by right-click

CHANGES.md

@@ -16,6 +16,7 @@ Fixed Issues:
 * [#1478](https://github.com/ckeditor/ckeditor-dev/issues/1478): Fixed: Custom colors added to [Color Button](https://ckeditor.com/cke4/addon/colorbutton) via [`config.colorButton_colors`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-colorButton_colors) in form label/code don't work correctly.
 * [#1469](https://github.com/ckeditor/ckeditor-dev/issues/1469): Fixed: Trying to get data from nested editable inside freshly pasted widget throws an error.
 * [#2923](https://github.com/ckeditor/ckeditor-dev/issues/2923): Fixed: CSS `windowtext` color is not correctly recognized by [`CKEDITOR.tools.style.parse`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_tools_style_parse.html) functions.
+* [#2235](https://github.com/ckeditor/ckeditor-dev/issues/2235): Fixed: [Image](https://ckeditor.com/cke4/addon/image) in table cell has an empty URL field when edited from context menu opened by right-click when [Table Selection](https://ckeditor.com/cke4/addon/tableselection) plugin is in use.
 
 API Changes:
 

core/selection.js

@@ -23,10 +23,12 @@
 		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;
 

plugins/tableselection/plugin.js

@@ -119,6 +119,7 @@
 
 	function clearFakeCellSelection( editor, reset ) {
 		var selectedCells = editor.editable().find( '.' + fakeSelectedClass ),
+			selectedTable = editor.editable().findOne( '[data-' + fakeSelectedTableDataAttribute + ']' ),
 			i;
 
 		editor.fire( 'lockSnapshot' );
@@ -129,8 +130,9 @@
 			selectedCells.getItem( i ).removeClass( fakeSelectedClass );
 		}
 
-		if ( selectedCells.count() > 0 ) {
-			selectedCells.getItem( 0 ).getAscendant( 'table' ).data( fakeSelectedTableDataAttribute, false );
+		// Table may be selected even though no cells are selected (e.g. after deleting cells.)
+		if ( selectedTable ) {
+			selectedTable.data( fakeSelectedTableDataAttribute, false );
 		}
 
 		editor.fire( 'unlockSnapshot' );
@@ -229,9 +231,11 @@
 		var editor = evt.editor || evt.sender.editor,
 			selection = editor && editor.getSelection(),
 			ranges = selection && selection.getRanges() || [],
+			enclosedNode = ranges && ranges[ 0 ].getEnclosedNode(),
+			isEnclosedNodeAnImage = enclosedNode && ( enclosedNode.type == CKEDITOR.NODE_ELEMENT ) && enclosedNode.is( 'img' ),
 			cells,
 			table,
-			i;
+			iterator;
 
 		if ( !selection ) {
 			return;
@@ -243,6 +247,12 @@
 			return;
 		}
 
+		// Don't perform fake selection when image is selected (#2235).
+		if ( isEnclosedNodeAnImage ) {
+			editor.getSelection().reset();
+			return;
+		}
+
 		// (#2945)
 		if ( ranges[ 0 ]._getTableElement( { table: 1 } ).hasAttribute( ignoredTableAttribute ) ) {
 			return;
@@ -259,8 +269,8 @@
 
 		editor.fire( 'lockSnapshot' );
 
-		for ( i = 0; i < cells.length; i++ ) {
-			cells[ i ].addClass( fakeSelectedClass );
+		for ( iterator = 0; iterator < cells.length; iterator++ ) {
+			cells[ iterator ].addClass( fakeSelectedClass );
 		}
 
 		if ( cells.length > 0 ) {

tests/plugins/tableselection/integrations/image/imageurl.html

@@ -0,0 +1,27 @@
+<div id="test">
+	<table border="1" cellpadding="5" cellspacing="5" style="width:500px">
+		<tbody>
+			<tr>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+			</tr>
+			<tr>
+				<td>
+				<table border="1" cellpadding="5" cellspacing="5" style="width:300px">
+					<tbody>
+						<tr>
+							<td>[<img alt="" src="%BASE_PATH%_assets/lena.jpg" style="height:100px; width:100px" />]</td>
+							<td>&nbsp;</td>
+						</tr>
+						<tr>
+							<td>&nbsp;</td>
+							<td>&nbsp;</td>
+						</tr>
+					</tbody>
+				</table>
+				</td>
+				<td>&nbsp;</td>
+			</tr>
+		</tbody>
+	</table>
+</div>

tests/plugins/tableselection/integrations/image/imageurl.js

@@ -0,0 +1,27 @@
+/* bender-tags: tableselection,2235,4.12.0 */
+/* bender-ckeditor-plugins: tableselection */
+/* bender-include: ../../_helpers/tableselection.js */
+/* global tableSelectionHelpers */
+
+( function() {
+	'use strict';
+
+	bender.editor = true;
+
+	var tests =  {
+		'Is whole cell fake selected when img inside is selected': function() {
+			var editor = this.editor,
+				bot = this.editorBot,
+				html = CKEDITOR.document.getById( 'test' ).getHtml();
+
+			bot.setHtmlWithSelection( html );
+
+			assert.isFalse( editor.getSelection().getSelectedElement().hasClass( 'cke_table-faked-selection' ) );
+		}
+	};
+
+	// Tests should be ignored in browsers which don't support tableselection plugin, i.e. IE < 11
+	tableSelectionHelpers.ignoreUnsupportedEnvironment( tests );
+	bender.test( tests );
+
+} )();

tests/plugins/tableselection/manual/integrations/image/nestedimage.html

@@ -0,0 +1,36 @@
+<div id="editor">
+	<table border="1" cellpadding="5" cellspacing="5" style="width:500px">
+		<tbody>
+			<tr>
+				<td>&nbsp;</td>
+				<td>&nbsp;</td>
+			</tr>
+			<tr>
+				<td>
+				<table border="1" cellpadding="5" cellspacing="5" style="width:300px">
+					<tbody>
+						<tr>
+							<td><img alt="" src="%BASE_PATH%_assets/lena.jpg" style="height:100px; width:100px"/></td>
+							<td>&nbsp;</td>
+						</tr>
+						<tr>
+							<td>&nbsp;</td>
+							<td>&nbsp;</td>
+						</tr>
+					</tbody>
+				</table>
+				<p>&nbsp;</p>
+				</td>
+				<td>&nbsp;</td>
+			</tr>
+		</tbody>
+	</table>
+</div>
+
+<script>
+	// Table Selection plugin doesn't work on IE < 11 so ignore test for that browsers.
+	if ( CKEDITOR.env.ie && CKEDITOR.env.version < 11 ) {
+		bender.ignore();
+	}
+	CKEDITOR.replace( 'editor', { height: 300 } );
+</script>

tests/plugins/tableselection/manual/integrations/image/nestedimage.md

@@ -0,0 +1,31 @@
+@bender-ui: collapsed
+@bender-tags: bug, 2235, 4.12.0
+@bender-ckeditor-plugins: wysiwygarea, tableselection, image, undo
+
+## Context menu for image in table
+
+### Scenario 1:
+
+1. Open context menu for image by right-clicking it.
+2. Choose `Image properties`.
+
+	**Expected result:**
+
+	URL field should be filled and image should be visible in the preview.
+
+	**Unexpected result:**
+
+	URL field is empty or image isn't visible in the preview.
+
+### Scenario 2:
+
+1. Select first row of the outer table and delete it.
+2. Click on the image again.
+
+	**Expected result:**
+
+	Image is selected.
+
+	**Unexpected result:**
+
+	Image can't be selected.