Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 23 commits into from
Jun 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8c75b12
Treat image selection like widget one.
Dumluregn Apr 15, 2019
9a8d3bb
Add manual test.
Dumluregn Apr 16, 2019
6f1c0e3
Add changelog entry.
Dumluregn Apr 16, 2019
3b4c389
Ignore test on browsers that don't support Table Selection plugin.
Dumluregn Apr 16, 2019
368def2
Remove redundant '.' character.
Dumluregn Apr 16, 2019
7fa51ed
Flatten the conditionals structure to one level.
Dumluregn Apr 16, 2019
8add1ab
Move widget and image detection from core file to plugin.
Dumluregn May 10, 2019
181d819
Refactor conditions into variables for readability.
Dumluregn May 10, 2019
309a248
Refactor again (fuse two logically identical scenarios).
Dumluregn May 10, 2019
6a42849
Remove 'fake selected' state from table (necessary after deleting som…
Dumluregn May 10, 2019
142a742
Add nested table and 'undo' plugin to test.
Dumluregn May 10, 2019
d0037e7
Make test table more tests-friendly.
Dumluregn May 10, 2019
44a4dcd
Move fake-selection removing to more logical place allowing to deal w…
Dumluregn May 10, 2019
18ff9ca
Reformat manual test instructions for readability.
Dumluregn May 20, 2019
19822ca
Move widget detection part back to the original file to avoid regress…
Dumluregn May 20, 2019
f96798a
Add unit test.
Dumluregn May 23, 2019
993884f
Reword test name a bit.
Dumluregn May 23, 2019
1ba7b51
Modify changelog entry due to retargeting branch to major.
Dumluregn May 23, 2019
6ea9757
Group all variable definitions.
Dumluregn May 23, 2019
c74c21a
Correct version tags.
Dumluregn May 24, 2019
b7a5c2b
Move early return earlier and change variable name to more verbose one.
Dumluregn May 24, 2019
44e5830
Fix the problematic fake selection for IE11+ and Edge.
Dumluregn May 27, 2019
d58712a
Delete unnecessary selection reset ruining the editor.
Dumluregn May 27, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
2 changes: 2 additions & 0 deletions core/selection.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@
if ( ranges.length === 0 ) {
return false;
}

Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
// It's not table selection when selected node is a widget (#1027).
if ( isWidget( ranges[ 0 ].getEnclosedNode() ) ) {
return false;
}

var node,
i;

Expand Down
20 changes: 15 additions & 5 deletions plugins/tableselection/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@

function clearFakeCellSelection( editor, reset ) {
var selectedCells = editor.editable().find( '.' + fakeSelectedClass ),
selectedTable = editor.editable().findOne( '[data-' + fakeSelectedTableDataAttribute + ']' ),
i;

editor.fire( 'lockSnapshot' );
Expand All @@ -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' );
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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 ) {
Expand Down
27 changes: 27 additions & 0 deletions tests/plugins/tableselection/integrations/image/imageurl.html
Original file line number Diff line number Diff line change
@@ -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>
27 changes: 27 additions & 0 deletions tests/plugins/tableselection/integrations/image/imageurl.js
Original file line number Diff line number Diff line change
@@ -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 );

} )();
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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.