diff --git a/CHANGES.md b/CHANGES.md index fce08835484..a34ebb3f64c 100644 --- a/CHANGES.md +++ b/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: diff --git a/core/selection.js b/core/selection.js index 750387fc536..383e5ac8a86 100644 --- a/core/selection.js +++ b/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 { diff --git a/plugins/tableselection/plugin.js b/plugins/tableselection/plugin.js index 22faf79714d..ad36bb89988 100644 --- a/plugins/tableselection/plugin.js +++ b/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,6 +274,11 @@ 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 ), @@ -276,8 +286,7 @@ 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 ); } diff --git a/plugins/widget/plugin.js b/plugins/widget/plugin.js index 7987b2386e2..40bc80dd3d4 100644 --- a/plugins/widget/plugin.js +++ b/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}. * diff --git a/tests/plugins/tableselection/integrations/image2/manual/deselectwidget.html b/tests/plugins/tableselection/integrations/image2/manual/deselectwidget.html new file mode 100644 index 00000000000..633ac9c4638 --- /dev/null +++ b/tests/plugins/tableselection/integrations/image2/manual/deselectwidget.html @@ -0,0 +1,34 @@ + + + diff --git a/tests/plugins/tableselection/integrations/image2/manual/deselectwidget.md b/tests/plugins/tableselection/integrations/image2/manual/deselectwidget.md new file mode 100644 index 00000000000..7b86c8c78b4 --- /dev/null +++ b/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. diff --git a/tests/plugins/widget/integration/table/_helpers/testwidgets.js b/tests/plugins/widget/integration/table/_helpers/testwidgets.js new file mode 100644 index 00000000000..099d4895c61 --- /dev/null +++ b/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: 'Very strong text.', + 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: '
Some text in div.
', + requiredContent: 'div(block-widget)', + allowedContent: 'div(!block-widget){*}', + upcast: function( element ) { + return element.name === 'div' && element.hasClass( 'block-widget' ); + } + } ); + } +} ); diff --git a/tests/plugins/widget/integration/table/manual/widgetintable.html b/tests/plugins/widget/integration/table/manual/widgetintable.html new file mode 100644 index 00000000000..d8dd2e9d51e --- /dev/null +++ b/tests/plugins/widget/integration/table/manual/widgetintable.html @@ -0,0 +1,77 @@ + + +
+ + + + + + + + + + + + + + + +
Some text in div.

 

Some text
+ + + + + + + + + + + +
Very strong text.Nested Table
+ 		Some text
+

 

+
+ + diff --git a/tests/plugins/widget/integration/table/manual/widgetintable.md b/tests/plugins/widget/integration/table/manual/widgetintable.md new file mode 100644 index 00000000000..c9b99dff455 --- /dev/null +++ b/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. diff --git a/tests/plugins/widget/integration/table/manual/widgetintablewithtableselection.html b/tests/plugins/widget/integration/table/manual/widgetintablewithtableselection.html new file mode 100644 index 00000000000..7fb5158d663 --- /dev/null +++ b/tests/plugins/widget/integration/table/manual/widgetintablewithtableselection.html @@ -0,0 +1,82 @@ + + +
+ + + + + + + + + + + + + + + +
Some text in div.

 

Some text
+ + + + + + + + + + + +
Very strong text.Nested Table
+ 		Some text
+

 

+
+ + diff --git a/tests/plugins/widget/integration/table/manual/widgetintablewithtableselection.md b/tests/plugins/widget/integration/table/manual/widgetintablewithtableselection.md new file mode 100644 index 00000000000..6a0af81ed17 --- /dev/null +++ b/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. diff --git a/tests/plugins/widget/widgetsrepoapi.js b/tests/plugins/widget/widgetsrepoapi.js index c28059ee00a..1fbcb331ea3 100644 --- a/tests/plugins/widget/widgetsrepoapi.js +++ b/tests/plugins/widget/widgetsrepoapi.js @@ -1701,6 +1701,12 @@ 'test Widget.isDomDragHandlerContainer - ': function() { assert.isTrue( Widget.isDomDragHandlerContainer( domEmWidgetDragHandlerContainer ) ); + }, + + 'test Widget.isDomWidget - all': function() { + assert.isFalse( Widget.isDomWidget( domEm ), 'domEm' ); + assert.isTrue( Widget.isDomWidget( domEmDataWidgetTest ), 'domEmDataWidgetTest' ); + assert.isTrue( Widget.isDomWidget( domEmDataWidgetWrapperTrue ), 'domEmDataWidgetWrapperTrue' ); } } ); } )();