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

Improve selection with widgetselection plugin when widget is partially selected #2866

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0555803
Tests: added unit test.
engineering-this Feb 20, 2019
25a22ac
Tests: fix typo in test name.
engineering-this Feb 25, 2019
88ae5e8
Tests: fixed on Firefox.
engineering-this Feb 25, 2019
1ebc537
Tests: ignore browsers whcih doesn't have this issue.
engineering-this Feb 26, 2019
93fb946
Tests: added comment with ticket number.
engineering-this Feb 26, 2019
eced316
Tests: added manual test.
engineering-this Feb 26, 2019
aa6883e
Tests: formatting.
engineering-this Feb 26, 2019
43384af
Tests: added ticket number reference.
engineering-this Mar 1, 2019
f230606
Tests: added test case.
engineering-this Apr 1, 2019
dfefbef
Tests: reworked.
engineering-this Apr 8, 2019
9876eb6
Tests: moved to widgetseleciton and renamed.
engineering-this Apr 9, 2019
df4e93b
Tests: add other ticket numbers, add test steps, remove float style, …
engineering-this Apr 9, 2019
051e4a6
Tests: rename function.
engineering-this Apr 9, 2019
78661a6
Tests: add comment.
engineering-this Apr 9, 2019
4c38be8
Tests: add test case.
engineering-this Apr 9, 2019
7fdac24
Tests: correct wrong env for ignoring mobiles.
engineering-this Apr 9, 2019
8748357
Tests: updated version tag.
engineering-this Apr 9, 2019
7e73f93
Tests: added manual test.
engineering-this Apr 9, 2019
c02f990
Tests: added test case.
engineering-this Apr 9, 2019
428c745
Fix seleciton when one end is in widget.
engineering-this Apr 9, 2019
15f6587
Fix typeerror by additional check in if statement.
engineering-this Apr 9, 2019
2ee520c
Tests: adjust indent level.
engineering-this Apr 9, 2019
719e0e4
Tests: extract lengthy duplicated html code into variable, adjust ind…
engineering-this Apr 9, 2019
6963390
Rename param.
engineering-this Apr 9, 2019
60b3cae
Replace big if with guard clause.
engineering-this Apr 9, 2019
0b386c4
Remove empty line.
engineering-this Apr 9, 2019
ca83c41
Changelog: add entries.
engineering-this Apr 9, 2019
523b15e
Tests: remove empty line.
engineering-this Apr 9, 2019
4d23a81
Tests: remove empty lines.
engineering-this Apr 9, 2019
0e7cc5c
Tests: added expected for IE.
engineering-this Apr 9, 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
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Fixed Issues:

* [#2672](https://github.com/ckeditor/ckeditor-dev/issues/2672): Fixed: When resizing [Enhanced Image](https://ckeditor.com/cke4/addon/image2) to minimum size with a resizer the image dialog doesn't show actual values.
* [#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.
* [#2517](https://github.com/ckeditor/ckeditor-dev/issues/2517), [#3007](https://github.com/ckeditor/ckeditor-dev/issues/3007), [#3008](https://github.com/ckeditor/ckeditor-dev/issues/3008): Fixed: various issues when widget is partially selected by prohibiting such selection with the [Widget Selection](https://ckeditor.com/cke4/addon/widgetselection) plugin.
* [#3041](https://github.com/ckeditor/ckeditor-dev/issues/3041): Fixed: [`range.deleteContents( true )`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dom_range.html#method-deleteContents) throws an error.

API Changes:

Expand Down
2 changes: 1 addition & 1 deletion core/dom/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ CKEDITOR.dom.range = function( root ) {
var topLeft = startParents[ commonLevel + 1 ];

// TopLeft may simply not exist if commonLevel == maxLevel or may be a text node.
if ( topLeft && topLeft.type == CKEDITOR.NODE_ELEMENT ) {
if ( topLeft && topLeft.type == CKEDITOR.NODE_ELEMENT && range.document.getDocumentElement().contains( topLeft ) ) {
var span = CKEDITOR.dom.element.createFromHtml( '<span ' +
'data-cke-bookmark="1" style="display:none">&nbsp;</span>', range.document );
span.insertAfter( topLeft );
Expand Down
69 changes: 63 additions & 6 deletions plugins/widgetselection/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@
CKEDITOR.plugins.add( 'widgetselection', {

init: function( editor ) {
if ( CKEDITOR.env.webkit ) {
var widgetselection = CKEDITOR.plugins.widgetselection;

editor.on( 'contentDom', function( evt ) {
editor.on( 'contentDom', function( evt ) {

var editor = evt.editor,
editable = editor.editable();
var editor = evt.editor,
editable = editor.editable();

if ( CKEDITOR.env.webkit ) {
var widgetselection = CKEDITOR.plugins.widgetselection;

editable.attachListener( editable, 'keydown', function( evt ) {
// Ctrl/Cmd + A
Expand Down Expand Up @@ -51,8 +52,64 @@
if ( 'selectall' in editor.plugins ) {
widgetselection.addSelectAllIntegration( editor );
}
}

// Normalize selection when only one end of range is in widget (#2517) (#3007) (#3008).
var selectionChangeListener = editor.on( 'selectionChange', selectionChangeCallback );

editable.getParent().getDocument().on( 'mousedown', function() {
selectionChangeListener.removeListener();

var mouseupListener = this.once( 'mouseup', function() {
fixRangesInWidgets();
mouseupListener.removeListener();
selectionChangeListener = editor.on( 'selectionChange', selectionChangeCallback );
} );
} );
}

function selectionChangeCallback( evt ) {
fixRangesInWidgets();
evt.cancel();
}

function fixRangesInWidgets() {
var selection = editor.getSelection(),
range = selection.getRanges()[ 0 ],
widget = CKEDITOR.plugins.widget,
isWidget = widget.isDomWidget.bind( widget ),
getByElement = editor.widgets.getByElement.bind( editor.widgets );

if ( selection.isFake || !range || selection.isCollapsed() ) {
return;
}

var startWidget = getByElement( range.startContainer.getAscendant( isWidget, true ) ),
endWidget = getByElement( range.endContainer.getAscendant( isWidget, true ) );

if ( startWidget !== endWidget ) {
fixSelectionInWidget( startWidget, 'start' );
fixSelectionInWidget( endWidget, 'end' );

range.select();
}

function fixSelectionInWidget( widget, prefix ) {
if ( !widget ) {
return;
}
var widgetOuter = widget.wrapper || widget.element,
container = range[ prefix + 'Container' ],
offset = range[ prefix + 'Offset' ],
limit = prefix === 'start' ? 0 : container.getChildCount();

if ( widgetOuter.equals( container ) && offset === limit ) {
range[ prefix === 'start' ? 'setStartBefore' : 'setEndAfter' ]( widgetOuter );
} else {
range[ prefix === 'start' ? 'setStartAfter' : 'setEndBefore' ]( widgetOuter );
}
}
}
} );
}
} );

Expand Down
19 changes: 19 additions & 0 deletions tests/core/dom/range/deletecontents.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,25 @@
range.deleteContents();

assert.isInnerHtmlMatching( '<h1></h1>[]<h2><br /></h2>', bender.tools.range.getWithHtml( root, range ) );
},

// (#3041)
'test deleteContents - range starts in wrapper, ends in second paragraph': function() {
var root = doc.createElement( 'div' ),
range = new CKEDITOR.dom.range( doc );

root.setAttribute( 'contenteditable', 'true' );
root.setHtml( '<p>foo</p><p>bar</p>' );

range.setStartAt( root, CKEDITOR.POSITION_AFTER_START );
range.setEndAt( root.getLast(), CKEDITOR.POSITION_BEFORE_END );

try {
range.deleteContents( true );
assert.pass();
} catch ( e ) {
assert.fail( e.message );
}
}
};

Expand Down
33 changes: 33 additions & 0 deletions tests/core/dom/range/manual/deletecontents.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<textarea id="editor">
<p>Foo</p>
<p>Bar</p>
</textarea>
<p id="controls">
<button data-button-action="select">Create range</button>
<button data-button-action="delete">Delete range contents</button>
<button data-button-action="restore">Restore editor</button>
</p>
<script>
if ( bender.tools.env.mobile ) {
bender.ignore();
}
var editor = CKEDITOR.replace( 'editor' );

CKEDITOR.document.findOne( '#controls' ).on( 'click', function( evt ) {
switch ( evt.data.getTarget().data( 'button-action' ) ) {
case 'select': return select();
case 'delete': return editor.getSelection().getRanges()[ 0 ].deleteContents( true );
case 'restore': editor.setData( '<p>Foo</p><p>Bar</p>' );
}

function select() {
var rng = editor.createRange(),
editable = editor.editable();

rng.setStartAt( editable, CKEDITOR.POSITION_AFTER_START );
rng.setEndAt( editable.getLast(), CKEDITOR.POSITION_BEFORE_END );
editor.focus();
rng.select();
}
} );
</script>
22 changes: 22 additions & 0 deletions tests/core/dom/range/manual/deletecontents.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
@bender-tags: 4.12.0, bug, 3041
@bender-ui: collapsed
@bender-ckeditor-plugins: widget, wysiwygarea, toolbar, sourcearea, image2, elementspath, undo, resize

1. Open developer console.
1. Press `Create range`.
1. Press `Delete range contents`.

## Expected
Editor content is removed.
## Unexpected
Error thrown

1. Press `Restore editor` and then `Create range`.
1. Open `image2` dialog.
1. Type any image URL, can be fake and press `OK`.

## Expected
Image is inserted into editor

## Unexpected
Error thrown
11 changes: 11 additions & 0 deletions tests/plugins/widgetselection/manual/nativeselection.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<textarea id="editor">
<figure class="image"><img height="50" src="../../../_assets/lena.jpg" /><figcaption>Caption</figcaption></figure>
<p>Foo</p>
</textarea>

<script>
if ( bender.tools.env.mobile ) {
bender.ignore();
}
CKEDITOR.replace( 'editor' );
</script>
32 changes: 32 additions & 0 deletions tests/plugins/widgetselection/manual/nativeselection.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
@bender-tags: 4.12.0, bug, 2517, 3007, 3008
@bender-ui: collapsed
@bender-ckeditor-plugins: widget, wysiwygarea, toolbar, sourcearea, image2, elementspath, undo, resize

1. Open browser console.
1. Start selecting text from right with mouse.
1. Release mouse over bottom of a widget.
1. Press <kbd>BACKSPACE</kbd>

- ## Expected
- Selected text is removed.
- [IE]: whole content is removed.

- ## Unexpected
- The widget caption is removed.
- Selection is in the widget caption.
- Selected text is not removed.

1. Press `Undo`.

- ## Unexpected
- `Image` button is disabled.

1. Press `Image` button.
1. Enter any image source (may be fake) and press `OK`.

## Expected
- Image is inserted.

## Unexpected
- Image isn't inserted.
- Error is thrown.
150 changes: 150 additions & 0 deletions tests/plugins/widgetselection/nativeselection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/* bender-tags: widgetcore */
/* bender-ckeditor-plugins: widget,widgetselection,toolbar,clipboard */

( function() {
'use strict';

bender.editors = {
classic: {
config: {
allowedContent: true
}
},
divarea: {
config: {
allowedContent: true,
extraPlugins: 'divarea'
}
}
};

var imageHtml = '<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAUEBAAAACwAAAAAAQABAAACAkQBADs=" />',
tests = {
// (#2517) (#3007)
'test selection when starts at the end of a widget': assertSelection( {
initial: '<div contenteditable="false"><div>FakeWidget</div>[</div>foo]',
expected: '<div contenteditable="false"><div>FakeWidget</div></div>[foo]'
} ),
// (#2517) (#3007)
'test selection when starts at the beginning of a widget': assertSelection( {
initial: '<div contenteditable="false">[<div>FakeWidget</div></div>foo]',
expected: '[<div contenteditable="false"><div>FakeWidget</div></div>foo]'
} ),
// (#2517) (#3007)
'test selection when ends at the beginning of a widget': assertSelection( {
initial: '[foo<div contenteditable="false">]<div>FakeWidget</div></div>',
expected: '[foo]<div contenteditable="false"><div>FakeWidget</div></div>'
} ),
// (#2517) (#3007)
'test selection when ends at the end of a widget': assertSelection( {
initial: '[foo<div contenteditable="false"><div>FakeWidget</div>]</div>',
expected: '[foo<div contenteditable="false"><div>FakeWidget</div></div>]'
} ),
// (#2517) (#3007)
'test selection when starts at the end and ends at the beginning of a widget': assertSelection( {
initial: '<div contenteditable="false"><div>FakeWidget</div>[</div>foo<div contenteditable="false">]<div>FakeWidget</div></div>',
expected: '<div contenteditable="false"><div>FakeWidget</div></div>[foo]<div contenteditable="false"><div>FakeWidget</div></div>'
} ),
// (#2517) (#3007)
'test selection when starts at the beginning and ends at the end of a widget': assertSelection( {
initial: '<div contenteditable="false">[<div>FakeWidget</div></div>foo<div contenteditable="false"><div>FakeWidget</div>]</div>',
expected: '[<div contenteditable="false"><div>FakeWidget</div></div>foo<div contenteditable="false"><div>FakeWidget</div></div>]'
} ),
// (#2517) (#3007)
'test selection when starts in the drag handler': assertSelection( {
initial: '<div contenteditable="false">' +
'<figure>' + imageHtml + '<figcaption>caption</figcaption></figure>' +
'<span>' + imageHtml + '[</span></div>foo]',
expected: '<div contenteditable="false">' +
'<figure>' + imageHtml + '<figcaption>caption</figcaption></figure>' +
'<span>' + imageHtml + '</span></div>[foo]'
} ),
// (#3008)
'test selection when starts in the nested editable': assertSelection( {
initial: '<div contenteditable="false">' +
'<figure>' + imageHtml + '<figcaption contenteditable="true">[caption</figcaption></figure>' +
'</div>foo]',
expected: '<div contenteditable="false">' +
'<figure>' + imageHtml + '<figcaption contenteditable="true">caption</figcaption></figure>' +
'</div>[foo]'
} )
};

bender.test( bender.tools.createTestsForEditors( CKEDITOR.tools.objectKeys( bender.editors ), tests ) );

function assertSelection( options ) {
return function( editor, bot ) {
bot.setHtmlWithSelection( options.initial );

var filter = new CKEDITOR.htmlParser.filter( {
text: function( value ) {
return value.replace( '{', '[' ).replace( '}', ']' );
}
} ),
elements = [],
wrappers = [],
current, fakeWidget;

filter.addRules( {
elements: {
br: remove
},
attributes: {
'class': remove,
'data-cke-widget': remove,
'data-cke-widget-wrapper': remove,
'data-cke-widget-id': remove
}
}, { applyToAll: true } ); // Options can be added only via `addRules`.

current = bender.tools.compatHtml( bender.tools.selection.getWithHtml( editor ), true, true, true, true, true, true, [ filter ] );

if ( current.replace( '{', '[' ) !== options.initial ) {
// Browser can't create tested selection.
assert.ignore();
}

// Mock widgets.
editor.editable().forEach( function( node ) {
if ( node.type === CKEDITOR.NODE_TEXT ) {
return;
}
if ( node.getHtml().replace( '<br>', '' ) === 'FakeWidget' ) {
node.data( 'cke-widget', 'true' );
elements.push( node );
} else if ( node.getAttribute( 'contenteditable' ) === 'false' ) {
node.data( 'cke-widget-wrapper', 'true' );
node.data( 'cke-widget-id', wrappers.length + 1 );
wrappers.push( node );
}
} );

createFakeWidget( 0 );
editor.widgets.instances = {
1: fakeWidget
};

if ( elements[ 1 ] ) {
createFakeWidget( 1 );
editor.widgets.instances[ 2 ] = fakeWidget;
}

editor.fire( 'selectionChange', {
selection: editor.getSelection(),
path: editor.elementPath()
} );

assert.beautified.html( options.expected, bender.tools.selection.getWithHtml( editor ), { customFilters: [ filter ] } );

function createFakeWidget( index ) {
fakeWidget = CKEDITOR.tools.copy( CKEDITOR.plugins.widget.prototype );
fakeWidget.element = elements[ index ];
fakeWidget.wrapper = wrappers[ index ];
}

function remove() {
return false;
}
};
}
} )();