Skip to content

Commit 36b41b2

Browse files
ReinmarReinmar
committed
Merge branch 't/13140b' into major
2 parents c59774c + 1ef3144 commit 36b41b2

File tree

6 files changed

+690
-96
lines changed

6 files changed

+690
-96
lines changed

‎CHANGES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Fixed Issues:
1919
* [#13036](http://dev.ckeditor.com/ticket/13036): Fixed: Notifications are 10px more to the right.
2020
* [#13280](http://dev.ckeditor.com/ticket/13280): [IE8] Fixed: Undo after inline widget drag&drop throws an error.
2121
* [#13186](http://dev.ckeditor.com/ticket/13186): Fixed: Content dropped to a nested editable is not filtered by the Advanced Content Filter.
22+
* [#13140](http://dev.ckeditor.com/ticket/13140): Fixed: Error thrown when dropping a block widget right after itself.
2223
* Toolbar configurators:
2324
* [#13185](http://dev.ckeditor.com/ticket/13185): Fixed: Wrong position of the suggestion box if there is not enough space below the caret.
2425
* [#13138](http://dev.ckeditor.com/ticket/13138): Fixed: The "Toggle empty elements" button label is unclear.

‎plugins/clipboard/plugin.js

Lines changed: 99 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/**
1+
/**
22
* @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
33
* For licensing, see LICENSE.md or http://ckeditor.com/license
44
*/
@@ -1371,61 +1371,18 @@
13711371
dataTransfer = data.dataTransfer;
13721372

13731373
if ( dataTransfer.getTransferType( editor ) == CKEDITOR.DATA_TRANSFER_INTERNAL ) {
1374-
internalDrop( dragRange, dropRange, dataTransfer );
1374+
// Execute drop with a timeout because otherwise selection, after drop,
1375+
// on IE is in the drag position, instead of drop position.
1376+
setTimeout( function() {
1377+
clipboard.internalDrop( dragRange, dropRange, dataTransfer, editor );
1378+
}, 0 );
13751379
} else if ( dataTransfer.getTransferType( editor ) == CKEDITOR.DATA_TRANSFER_CROSS_EDITORS ) {
13761380
crossEditorDrop( dragRange, dropRange, dataTransfer );
13771381
} else {
13781382
externalDrop( dropRange, dataTransfer );
13791383
}
13801384
}, null, null, 9999 );
13811385

1382-
// Internal drag and drop (drag and drop in the same editor).
1383-
function internalDrop( dragRange, dropRange, dataTransfer ) {
1384-
// Execute drop with a timeout because otherwise selection, after drop,
1385-
// on IE is in the drag position, instead of drop position.
1386-
setTimeout( function() {
1387-
var dragBookmark, dropBookmark, isRangeBefore;
1388-
1389-
// Save and lock snapshot so there will be only
1390-
// one snapshot for both remove and insert content.
1391-
editor.fire( 'saveSnapshot' );
1392-
editor.fire( 'lockSnapshot', { dontUpdate: 1 } );
1393-
1394-
if ( CKEDITOR.env.ie && CKEDITOR.env.version < 10 ) {
1395-
clipboard.fixIESplitNodesAfterDrop( dragRange, dropRange );
1396-
}
1397-
1398-
// Because we manipulate multiple ranges we need to do it carefully,
1399-
// changing one range (event creating a bookmark) may make other invalid.
1400-
// We need to change ranges into bookmarks so we can manipulate them easily in the future.
1401-
// We can change the range which is later in the text before we change the preceding range.
1402-
// We call isRangeBefore to test the order of ranges.
1403-
isRangeBefore = clipboard.isRangeBefore( dragRange, dropRange );
1404-
if ( !isRangeBefore ) {
1405-
dragBookmark = dragRange.createBookmark( 1 );
1406-
}
1407-
dropBookmark = dropRange.clone().createBookmark( 1 );
1408-
if ( isRangeBefore ) {
1409-
dragBookmark = dragRange.createBookmark( 1 );
1410-
}
1411-
1412-
// No we can safely delete content for the drag range...
1413-
dragRange = editor.createRange();
1414-
dragRange.moveToBookmark( dragBookmark );
1415-
editable.extractHtmlFromRange( dragRange, 1 );
1416-
1417-
// ...and paste content into the drop position.
1418-
dropRange = editor.createRange();
1419-
dropRange.moveToBookmark( dropBookmark );
1420-
1421-
// We do not select drop range, because of may be in the place we can not set the selection
1422-
// (e.g. between blocks, in case of block widget D&D). We put range to the paste event instead.
1423-
firePasteEvents( editor, { dataTransfer: dataTransfer, method: 'drop', range: dropRange }, 1 );
1424-
1425-
editor.fire( 'unlockSnapshot' );
1426-
}, 0 );
1427-
}
1428-
14291386
// Cross editor drag and drop (drag in one Editor and drop in the other).
14301387
function crossEditorDrop( dragRange, dropRange, dataTransfer ) {
14311388
// Paste event should be fired before delete contents because otherwise
@@ -1582,43 +1539,114 @@
15821539
},
15831540

15841541
/**
1585-
* Check if the beginning of the `firstRange` is before the beginning of the `secondRange`
1586-
* and modification of the content in the `firstRange` may break `secondRange`.
1542+
* Checkes whether turning drag range into bookmarks will invalidate the drop range.
1543+
* This usually happens when drop range shares the container with drag range and is located
1544+
* after the drag range, but there are countless edge cases.
15871545
*
1588-
* Notify that this function returns `false` if these two ranges are in two
1589-
* separate nodes and do not affect each other (even if `firstRange` is before `secondRange`).
1546+
* This function is stricly related to {@link #internalDrop} which toggles
1547+
* order in which it creates bookmarks for both ranges based on a value returned
1548+
* by this method. In some cases this method returns a value which does not necessarily
1549+
* is true in terms of what it was meant to check, but it is convenient, because
1550+
* we know how it is interpretted in {@link #internalDrop}, so the correct
1551+
* behavior of the entire algorithm is assured.
15901552
*
15911553
* **Note:** This function is in the public scope for tests usage only.
15921554
*
15931555
* @since 4.5
15941556
* @private
1595-
* @param {CKEDITOR.dom.range} firstRange The first range to compare.
1596-
* @param {CKEDITOR.dom.range} secondRange The second range to compare.
1557+
* @param {CKEDITOR.dom.range} dragRange The first range to compare.
1558+
* @param {CKEDITOR.dom.range} dropRange The second range to compare.
15971559
* @returns {Boolean} True if the first range in before the second range.
15981560
*/
1599-
isRangeBefore: function( firstRange, secondRange ) {
1600-
// Both ranges has the same parent and the first has smaller offset. E.g.:
1601-
//
1602-
// "Lorem ipsum dolor sit[1] amet consectetur[2] adipiscing elit."
1603-
// "Lorem ipsum dolor sit" [1] "amet consectetur" [2] "adipiscing elit."
1604-
//
1605-
if ( firstRange.endContainer.equals( secondRange.startContainer ) &&
1606-
firstRange.endOffset < secondRange.startOffset )
1561+
isDropRangeAffectedByDragRange: function( dragRange, dropRange ) {
1562+
var dropContainer = dropRange.startContainer,
1563+
dropOffset = dropRange.endOffset;
1564+
1565+
// Both containers are the same and drop offset is at the same position or later.
1566+
// " A L] A " " M A "
1567+
// ^ ^
1568+
if ( dragRange.endContainer.equals( dropContainer ) && dragRange.endOffset <= dropOffset ) {
16071569
return true;
1570+
}
16081571

1609-
// First range is inside a text node and the second is not, but if we change the
1610-
// first range into bookmark and split the text node then the seconds node offset
1611-
// will be no longer correct.
1612-
//
1613-
// "Lorem ipsum dolor sit [1] amet" "consectetur" [2] "adipiscing elit."
1614-
//
1615-
if ( firstRange.endContainer.getParent().equals( secondRange.startContainer ) &&
1616-
firstRange.endContainer.getIndex() < secondRange.startOffset )
1572+
// Bookmark for drag start container will mess up with offsets.
1573+
// " O [L A " " M A "
1574+
// ^ ^
1575+
if (
1576+
dragRange.startContainer.getParent().equals( dropContainer ) &&
1577+
dragRange.startContainer.getIndex() < dropOffset
1578+
) {
1579+
return true;
1580+
}
1581+
1582+
// Bookmark for drag end container will mess up with offsets.
1583+
// " O] L A " " M A "
1584+
// ^ ^
1585+
if (
1586+
dragRange.endContainer.getParent().equals( dropContainer ) &&
1587+
dragRange.endContainer.getIndex() < dropOffset
1588+
) {
16171589
return true;
1590+
}
16181591

16191592
return false;
16201593
},
16211594

1595+
/**
1596+
* Internal drag and drop (drag and drop in the same editor).
1597+
*
1598+
* **Note:** This function is in the public scope for tests usage only.
1599+
*
1600+
* @since 4.5
1601+
* @private
1602+
* @param {CKEDITOR.dom.range} dragRange The first range to compare.
1603+
* @param {CKEDITOR.dom.range} dropRange The second range to compare.
1604+
* @param {CKEDITOR.plugins.clipboard.dataTransfer} dataTransfer
1605+
* @param {CKEDITOR.editor} editor
1606+
*/
1607+
internalDrop: function( dragRange, dropRange, dataTransfer, editor ) {
1608+
var editable = editor.editable(),
1609+
dragBookmark, dropBookmark, isDropRangeAffected;
1610+
1611+
// Save and lock snapshot so there will be only
1612+
// one snapshot for both remove and insert content.
1613+
editor.fire( 'saveSnapshot' );
1614+
editor.fire( 'lockSnapshot', { dontUpdate: 1 } );
1615+
1616+
if ( CKEDITOR.env.ie && CKEDITOR.env.version < 10 ) {
1617+
this.fixIESplitNodesAfterDrop( dragRange, dropRange );
1618+
}
1619+
1620+
// Because we manipulate multiple ranges we need to do it carefully,
1621+
// changing one range (event creating a bookmark) may make other invalid.
1622+
// We need to change ranges into bookmarks so we can manipulate them easily in the future.
1623+
// We can change the range which is later in the text before we change the preceding range.
1624+
// We call isDropRangeAffectedByDragRange to test the order of ranges.
1625+
isDropRangeAffected = this.isDropRangeAffectedByDragRange( dragRange, dropRange );
1626+
if ( !isDropRangeAffected ) {
1627+
dragBookmark = dragRange.createBookmark( 1 );
1628+
}
1629+
dropBookmark = dropRange.clone().createBookmark( 1 );
1630+
if ( isDropRangeAffected ) {
1631+
dragBookmark = dragRange.createBookmark( 1 );
1632+
}
1633+
1634+
// No we can safely delete content for the drag range...
1635+
dragRange = editor.createRange();
1636+
dragRange.moveToBookmark( dragBookmark );
1637+
editable.extractHtmlFromRange( dragRange, 1 );
1638+
1639+
// ...and paste content into the drop position.
1640+
dropRange = editor.createRange();
1641+
dropRange.moveToBookmark( dropBookmark );
1642+
1643+
// We do not select drop range, because of may be in the place we can not set the selection
1644+
// (e.g. between blocks, in case of block widget D&D). We put range to the paste event instead.
1645+
firePasteEvents( editor, { dataTransfer: dataTransfer, method: 'drop', range: dropRange }, 1 );
1646+
1647+
editor.fire( 'unlockSnapshot' );
1648+
},
1649+
16221650
/**
16231651
* Get range from the `drop` event.
16241652
*

‎tests/plugins/clipboard/drop.js

Lines changed: 92 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -636,31 +636,31 @@ var testsForMultipleEditor = {
636636
assert.isInnerHtmlMatching( '<p id="p">lorem^ ipsum sit amet.@</p>', getWithHtml( editor ), htmlMatchOpts );
637637
},
638638

639-
'test isRangeBefore 1': function() {
639+
'test isDropRangeAffectedByDragRange 1': function() {
640640
var editor = this.editors.framed,
641641
bot = this.editorBots[ editor.name ],
642-
firstRange = editor.createRange(),
643-
secondRange = editor.createRange(),
642+
dragRange = editor.createRange(),
643+
dropRange = editor.createRange(),
644644
p;
645645

646646
// "Lorem[1] ipsum[2] sit amet."
647647
bot.setHtmlWithSelection( '<p id="p">Lorem ipsum sit amet.</p>' );
648648
p = editor.document.getById( 'p' );
649649

650-
firstRange.setStart( p.getChild( 0 ), 5 );
651-
firstRange.collapse( true );
650+
dragRange.setStart( p.getChild( 0 ), 5 );
651+
dragRange.collapse( true );
652652

653-
secondRange.setStart( p.getChild( 0 ), 11 );
654-
secondRange.collapse( true );
653+
dropRange.setStart( p.getChild( 0 ), 11 );
654+
dropRange.collapse( true );
655655

656-
assert.isTrue( CKEDITOR.plugins.clipboard.isRangeBefore( firstRange, secondRange ) );
656+
assert.isTrue( CKEDITOR.plugins.clipboard.isDropRangeAffectedByDragRange( dragRange, dropRange ) );
657657
},
658658

659-
'test isRangeBefore 2': function() {
659+
'test isDropRangeAffectedByDragRange 2': function() {
660660
var editor = this.editors.framed,
661661
bot = this.editorBots[ editor.name ],
662-
firstRange = editor.createRange(),
663-
secondRange = editor.createRange(),
662+
dragRange = editor.createRange(),
663+
dropRange = editor.createRange(),
664664
p, text;
665665

666666
// "Lorem " [1] " ipsum" [2] "sit amet."
@@ -671,20 +671,20 @@ var testsForMultipleEditor = {
671671
text = new CKEDITOR.dom.text( ' sit amet.' );
672672
text.insertAfter( p.getChild( 0 ) );
673673

674-
firstRange.setStart( p, 1 );
675-
firstRange.collapse( true );
674+
dragRange.setStart( p, 1 );
675+
dragRange.collapse( true );
676676

677-
secondRange.setStart( p, 2 );
678-
secondRange.collapse( true );
677+
dropRange.setStart( p, 2 );
678+
dropRange.collapse( true );
679679

680-
assert.isTrue( CKEDITOR.plugins.clipboard.isRangeBefore( firstRange, secondRange ) );
680+
assert.isTrue( CKEDITOR.plugins.clipboard.isDropRangeAffectedByDragRange( dragRange, dropRange ) );
681681
},
682682

683-
'test isRangeBefore 3': function() {
683+
'test isDropRangeAffectedByDragRange 3': function() {
684684
var editor = this.editors.framed,
685685
bot = this.editorBots[ editor.name ],
686-
firstRange = editor.createRange(),
687-
secondRange = editor.createRange(),
686+
dragRange = editor.createRange(),
687+
dropRange = editor.createRange(),
688688
p, text;
689689

690690
// "Lorem[1] ipsum" [2] "sit amet."
@@ -693,13 +693,54 @@ var testsForMultipleEditor = {
693693
text = new CKEDITOR.dom.text( ' sit amet.' );
694694
text.insertAfter( p.getChild( 0 ) );
695695

696-
firstRange.setStart( p.getChild( 0 ), 5 );
697-
firstRange.collapse( true );
696+
dragRange.setStart( p.getChild( 0 ), 5 );
697+
dragRange.collapse( true );
698+
699+
dropRange.setStart( p, 1 );
700+
dropRange.collapse( true );
701+
702+
assert.isTrue( CKEDITOR.plugins.clipboard.isDropRangeAffectedByDragRange( dragRange, dropRange ) );
703+
},
704+
705+
'test isDropRangeAffectedByDragRange 4': function() {
706+
var editor = this.editors.framed,
707+
bot = this.editorBots[ editor.name ],
708+
dragRange = editor.createRange(),
709+
dropRange = editor.createRange(),
710+
p, text;
711+
712+
// <p> [2] "Lorem[1] ipsum" "sit amet." </p>
713+
bot.setHtmlWithSelection( '<p id="p">Lorem ipsum</p>' );
714+
p = editor.document.getById( 'p' );
715+
text = new CKEDITOR.dom.text( ' sit amet.' );
716+
text.insertAfter( p.getChild( 0 ) );
717+
718+
dragRange.setStart( p.getChild( 0 ), 5 );
719+
dragRange.collapse( true );
720+
721+
dropRange.setStart( p, 0 );
722+
dropRange.collapse( true );
723+
724+
assert.isFalse( CKEDITOR.plugins.clipboard.isDropRangeAffectedByDragRange( dragRange, dropRange ) );
725+
},
726+
727+
'test isDropRangeAffectedByDragRange adjacent positions (#13140)': function() {
728+
var editor = this.editors.framed,
729+
bot = this.editorBots[ editor.name ],
730+
dragRange = editor.createRange(),
731+
dropRange = editor.createRange(),
732+
div;
698733

699-
secondRange.setStart( p, 1 );
700-
secondRange.collapse( true );
734+
bot.setHtmlWithSelection( '<div><p id="foo">foo</p><p id="bar">bar</p></div>' );
735+
div = editor.document.findOne( 'div' );
701736

702-
assert.isTrue( CKEDITOR.plugins.clipboard.isRangeBefore( firstRange, secondRange ) );
737+
dragRange.setStart( div, 1 );
738+
dragRange.setEnd( div, 2 );
739+
740+
dropRange.setStart( div, 2 );
741+
dropRange.setEnd( div, 2 );
742+
743+
assert.isTrue( CKEDITOR.plugins.clipboard.isDropRangeAffectedByDragRange( dragRange, dropRange ) );
703744
},
704745

705746
'test dragEnd event': function() {
@@ -817,6 +858,32 @@ var testsForMultipleEditor = {
817858
} );
818859
},
819860

861+
'test drop block element at the same position': function() {
862+
var editor = this.editors.framed,
863+
evt = bender.tools.mockDropEvent();
864+
865+
bender.tools.selection.setWithHtml(
866+
editor,
867+
'<p>x</p>' +
868+
'[<p id="middle" contenteditable="false">middle</p>]' +
869+
'<p>y</p>'
870+
);
871+
872+
drag( editor, evt );
873+
drop( editor, evt, {
874+
element: editor.editable(),
875+
offset: 2,
876+
expectedPasteEventCount: 0
877+
}, function() {
878+
return false;
879+
}, function() {
880+
assert.isInnerHtmlMatching(
881+
'<p>x</p><p contenteditable="false" id="middle">middle</p><p>y</p>',
882+
editor.editable().getHtml()
883+
);
884+
} );
885+
},
886+
820887
'test set custom data on drop': function() {
821888
var editor = this.editors.inline,
822889
editable = editor.editable(),
@@ -887,4 +954,4 @@ bender.test(
887954
[ 'framed', 'inline', 'divarea' ], testsForMultipleEditor
888955
), testsForOneEditor
889956
)
890-
);
957+
);

0 commit comments

Comments
 (0)