Merge branch 't/13128d' into major

CHANGES.md

@@ -23,6 +23,10 @@ Fixed Issues:
 * [#13140](http://dev.ckeditor.com/ticket/13140): Fixed: Error thrown when dropping a block widget right after itself.
 * [#13176](http://dev.ckeditor.com/ticket/13176): [IE8] Fixed: Errors while drag&drop of embed widgets.
 * [#13015](http://dev.ckeditor.com/ticket/13015): Fixed: Dropping image file on [Enhanced Image](http://ckeditor.com/addon/image2) causes page reload.
+* [#13128](http://dev.ckeditor.com/ticket/13128): Fixed: Various issues regarding cloning IDs of elements:
+  * Fixed the default behavior of [`range.cloneContents()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.range-method-cloneContents), [`range.extractContents()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.range-method-extractContents) methods which now clone IDs similarly to their native counterparts.
+  * Added `cloneId` arguments to the above methods and [`range.splitBlock()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.range-method-splitBlock) and [`element.breakParent()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.element-method-breakParent). Mind the default values and special behavior in the `extracContents()` method.
+  * Fixed issues where IDs were lost when copy&paste and drag&drop.
 * Toolbar configurators:
   * [#13185](http://dev.ckeditor.com/ticket/13185): Fixed: Wrong position of the suggestion box if there is not enough space below the caret.
   * [#13138](http://dev.ckeditor.com/ticket/13138): Fixed: The "Toggle empty elements" button label is unclear.

core/dom/element.js

@@ -319,8 +319,9 @@ CKEDITOR.dom.element.clearMarkers = function( database, element, removeFromDatab
 		 *		element.breakParent( parent );
 		 *
 		 * @param {CKEDITOR.dom.element} parent The anscestor element to get broken.
+		 * @param {Boolean} [cloneId=false] Whether to preserve ancestor ID attributes while breaking.
 		 */
-		breakParent: function( parent ) {
+		breakParent: function( parent, cloneId ) {
 			var range = new CKEDITOR.dom.range( this.getDocument() );
 
 			// We'll be extracting part of this element, so let's use our
@@ -329,7 +330,7 @@ CKEDITOR.dom.element.clearMarkers = function( database, element, removeFromDatab
 			range.setEndAfter( parent );
 
 			// Extract it.
-			var docFrag = range.extractContents();
+			var docFrag = range.extractContents( false, cloneId || false );
 
 			// Move the element outside the broken element.
 			range.insertNode( this.remove() );

core/dom/range.js

@@ -169,11 +169,13 @@ CKEDITOR.dom.range = function( root ) {
 	//   * Then we do the same with end node parents (right branch), because it may contains notes we omit during the previous
 	//   step, for example if the right branch is deeper then left branch. Things are more complicated here because we have to
 	//   watch out for nodes that were already cloned.
+	//   * ***Note:** Setting `cloneId` option to `false` for **extraction** works for partially selected elements only.
+	//   See range.extractContents to know more.
 	// 4. Clean up.
 	//   * There are two things we need to do - updating the range position and perform the action of the "mergeThen"
 	//   param (see range.deleteContents or range.extractContents).
 	//   See comments in mergeAndUpdate because this is lots of fun too.
-	function execContentsAction( range, action, docFrag, mergeThen ) {
+	function execContentsAction( range, action, docFrag, mergeThen, cloneId ) {
 		'use strict';
 
 		range.optimizeBookmark();
@@ -301,7 +303,7 @@ CKEDITOR.dom.range = function( root ) {
 					levelParent.append( range.document.createText( leftNode.substring( startOffset ) ) );
 				}
 			} else if ( doClone ) {
-				nextLevelParent = levelParent.append( leftNode.clone() );
+				nextLevelParent = levelParent.append( leftNode.clone( 0, cloneId ) );
 			}
 
 			// 2.
@@ -358,7 +360,7 @@ CKEDITOR.dom.range = function( root ) {
 						levelParent.append( range.document.createText( rightNode.substring( 0, endOffset ) ) );
 					}
 				} else if ( doClone ) {
-					nextLevelParent = levelParent.append( rightNode.clone() );
+					nextLevelParent = levelParent.append( rightNode.clone( 0, cloneId ) );
 				}
 
 				// 2.
@@ -395,7 +397,7 @@ CKEDITOR.dom.range = function( root ) {
 
 			// If cloning, just clone it.
 			if ( isClone || forceClone ) {
-				newParent.append( node.clone( true ), toStart );
+				newParent.append( node.clone( true, cloneId ), toStart );
 			} else {
 				// Both Delete and Extract will remove the node.
 				node.remove();
@@ -632,13 +634,16 @@ CKEDITOR.dom.range = function( root ) {
 		/**
 		 * The content nodes of the range are cloned and added to a document fragment, which is returned.
 		 *
+		 * @param {Boolean} [cloneId=true] Whether to preserve ID attributes in the clone.
 		 * @returns {CKEDITOR.dom.documentFragment} Document fragment containing clone of range's content.
 		 */
-		cloneContents: function() {
+		cloneContents: function( cloneId ) {
 			var docFrag = new CKEDITOR.dom.documentFragment( this.document );
 
+			cloneId = typeof cloneId == 'undefined' ? true : cloneId;
+
 			if ( !this.collapsed )
-				execContentsAction( this, 2, docFrag );
+				execContentsAction( this, 2, docFrag, false, cloneId );
 
 			return docFrag;
 		},
@@ -659,14 +664,21 @@ CKEDITOR.dom.range = function( root ) {
 		 * The content nodes of the range are cloned and added to a document fragment,
 		 * meanwhile they are removed permanently from the DOM tree.
 		 *
-		 * @param {Boolean} [mergeThen] Merge any splitted elements result in DOM true due to partial selection.
+		 * **Note:** Setting `cloneId` option to `false` works for **partially** selected elements only.
+		 * If an element with an ID attribute is **fully enclosed** in a range, it will keep the ID attribute
+		 * regardless of `cloneId` option value, because it is not cloned – it is moved to the returned document fragment.
+		 *
+		 * @param {Boolean} [mergeThen] Merge any split elements result in DOM true due to partial selection.
+		 * @param {Boolean} [cloneId=true] Whether to preserve ID attributes in the extracted content.
 		 * @returns {CKEDITOR.dom.documentFragment} Document fragment containing extracted content.
 		 */
-		extractContents: function( mergeThen ) {
+		extractContents: function( mergeThen, cloneId ) {
 			var docFrag = new CKEDITOR.dom.documentFragment( this.document );
 
+			cloneId = typeof cloneId == 'undefined' ? true : cloneId;
+
 			if ( !this.collapsed )
-				execContentsAction( this, 1, docFrag, mergeThen );
+				execContentsAction( this, 1, docFrag, mergeThen, cloneId );
 
 			return docFrag;
 		},
@@ -2070,8 +2082,9 @@ CKEDITOR.dom.range = function( root ) {
 
 		/**
 		 * @todo
+		 * @param {Boolean} [cloneId=false] Whether to preserve ID attributes in the result blocks.
 		 */
-		splitBlock: function( blockTag ) {
+		splitBlock: function( blockTag, cloneId ) {
 			var startPath = new CKEDITOR.dom.elementPath( this.startContainer, this.root ),
 				endPath = new CKEDITOR.dom.elementPath( this.endContainer, this.root );
 
@@ -2115,7 +2128,7 @@ CKEDITOR.dom.range = function( root ) {
 					this.moveToPosition( startBlock, CKEDITOR.POSITION_BEFORE_START );
 					startBlock = null;
 				} else {
-					endBlock = this.splitElement( startBlock );
+					endBlock = this.splitElement( startBlock, cloneId || false );
 
 					// In Gecko, the last child node must be a bogus <br>.
 					// Note: bogus <br> added under <ul> or <ol> would cause
@@ -2141,19 +2154,20 @@ CKEDITOR.dom.range = function( root ) {
 		 * **Note:** The range must be collapsed and been enclosed by this element.
 		 *
 		 * @param {CKEDITOR.dom.element} element
+		 * @param {Boolean} [cloneId=false] Whether to preserve ID attributes in the result elements.
 		 * @returns {CKEDITOR.dom.element} Root element of the new branch after the split.
 		 */
-		splitElement: function( toSplit ) {
+		splitElement: function( toSplit, cloneId ) {
 			if ( !this.collapsed )
 				return null;
 
 			// Extract the contents of the block from the selection point to the end
 			// of its contents.
 			this.setEndAt( toSplit, CKEDITOR.POSITION_BEFORE_END );
-			var documentFragment = this.extractContents();
+			var documentFragment = this.extractContents( false, cloneId || false );
 
 			// Duplicate the element after it.
-			var clone = toSplit.clone( false );
+			var clone = toSplit.clone( false, cloneId || false );
 
 			// Place the extracted contents into the duplicated element.
 			documentFragment.appendTo( clone );

tests/core/dom/element/element.html

@@ -92,7 +92,7 @@
 	<div id="domObjectTest1"></div>
 	<div id="domObjectTest2"></div>
 	<div id="appendHtml"></div>
-	<div id="breakParent"><i>text1<b>text2</b>text3</i></div>
+	<div id="breakParent"><i id="a">text1<b id="b">text2</b>text3</i></div>
 	<div id="contains"><p id="contains1">text</p></div>
 	<div id="getChildren"><i></i><b></b>text</div>
 	<div id="isIdentical"><b name="a" class="test">tessst</b><b class="test" name="a"></b><b class="test"></b><b>tessst</b><b _moz_dirty="" class="test"></b><b class="test" data-cke-expando=53></b></div>

tests/core/dom/element/element.js

@@ -657,7 +657,7 @@ bender.test( appendDomObjectTests(
 
 			element.breakParent( parent );
 
-			assert.areEqual( '<i>text1</i><b>text2</b><i>text3</i>', getInnerHtml( 'breakParent' ) );
+			assert.areEqual( '<i id="a">text1</i><b id="b">text2</b><i>text3</i>', getInnerHtml( 'breakParent' ) );
 		},
 
 		test_contains: function() {

tests/core/dom/range/clonecontents.js

@@ -32,7 +32,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( 'his is <b>some</b>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
+			assert.areSame( 'his is <b id="_b">some</b>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
 
 			// The body HTML must remain unchanged.
 			assert.areSame( bodyHtml.replace( /\s+_cke_expando=["\d]+/g, '' ), document.getElementById( 'playground' ).innerHTML.replace( /\s+_cke_expando=["\d]+/g, '' ), 'The HTML must remain untouched' );
@@ -59,7 +59,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( '<b>ome</b> t', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
+			assert.areSame( '<b id="_b">ome</b> t', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
 
 			// The body HTML must remain unchanged.
 			assert.areSame( bodyHtml.replace( /\s+_cke_expando=["\d]+/g, '' ), document.getElementById( 'playground' ).innerHTML.replace( /\s+_cke_expando=["\d]+/g, '' ), 'The HTML must remain untouched' );
@@ -86,7 +86,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( 'his is <b>s</b>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
+			assert.areSame( 'his is <b id="_b">s</b>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
 
 			// The body HTML must remain unchanged.
 			assert.areSame( bodyHtml.replace( /\s+_cke_expando=["\d]+/g, '' ), document.getElementById( 'playground' ).innerHTML.replace( /\s+_cke_expando=["\d]+/g, '' ), 'The HTML must remain untouched' );
@@ -113,7 +113,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( '<h1>ckw3crange test</h1><p>this is <b>some</b> text.</p><p>a</p>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
+			assert.areSame( '<h1 id="_h1">ckw3crange test</h1><p id="_para">this is <b id="_b">some</b> text.</p><p>a</p>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
 
 			// The body HTML must remain unchanged.
 			assert.areSame( bodyHtml.replace( /\s+_cke_expando=["\d]+/g, '' ), document.getElementById( 'playground' ).innerHTML.replace( /\s+_cke_expando=["\d]+/g, '' ), 'The HTML must remain untouched' );
@@ -139,7 +139,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( '<h1>fckw3crange test</h1><p>this is <b>some</b> text.</p><p>another paragraph.</p>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
+			assert.areSame( '<h1 id="_h1">fckw3crange test</h1><p id="_para">this is <b id="_b">some</b> text.</p><p>another paragraph.</p>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
 
 			// The body HTML must remain unchanged.
 			assert.areSame( bodyHtml.replace( /\s+_cke_expando=["\d]+/g, '' ), document.getElementById( 'playground' ).innerHTML.replace( /\s+_cke_expando=["\d]+/g, '' ), 'The HTML must remain untouched' );
@@ -165,7 +165,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( '<h1>fckw3crange test</h1><p>this is <b>some</b> text.</p>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
+			assert.areSame( '<h1 id="_h1">fckw3crange test</h1><p id="_para">this is <b id="_b">some</b> text.</p>', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
 
 			// The body HTML must remain unchanged.
 			assert.areSame( bodyHtml.replace( /\s+_cke_expando=["\d]+/g, '' ), document.getElementById( 'playground' ).innerHTML.replace( /\s+_cke_expando=["\d]+/g, '' ), 'The HTML must remain untouched' );
@@ -214,7 +214,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( 'this is <b>some</b> text.', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
+			assert.areSame( 'this is <b id="_b">some</b> text.', getInnerHtml( tmpDiv.$ ), 'Cloned HTML' );
 
 			// The body HTML must remain unchanged.
 			assert.areSame( bodyHtml.replace( /\s+_cke_expando=["\d]+/g, '' ), document.getElementById( 'playground' ).innerHTML.replace( /\s+_cke_expando=["\d]+/g, '' ), 'The HTML must remain untouched' );
@@ -490,6 +490,30 @@
 			var clone = range.cloneContents();
 
 			assert.isInnerHtmlMatching( '<h1></h1><h2></h2>', clone.getHtml() );
+		},
+
+		'test cloneContents - element ID handling': function() {
+			var root = doc.createElement( 'div' ),
+				range = new CKEDITOR.dom.range( doc );
+
+			root.setHtml( '<div><p id="x">f<b id="y">o</b>o</p></div>' );
+			doc.getBody().append( root );
+
+			// <div>[<p id="x">f<b id="y">o</b>o</p>]</div>
+			range.setStartBefore( root.findOne( 'p' ) );
+			range.setEndAfter( root.findOne( 'p' ) );
+
+			var cloneWithId = range.cloneContents(),
+				cloneWithoutId = range.cloneContents( false );
+
+			assert.isInnerHtmlMatching( '<p id="x">f<b id="y">o</b>o</p>', cloneWithId.getHtml(), 'Clone with IDs.' );
+			assert.isInnerHtmlMatching( '<p>f<b>o</b>o</p>', cloneWithoutId.getHtml(), 'Clone without IDs.' );
+
+			assert.isInnerHtmlMatching( '<div><p id="x">f<b id="y">o</b>o</p></div>', root.getHtml() );
+			assert.areSame( root.findOne( 'div' ), range.startContainer, 'range.startContainer' );
+			assert.areSame( root.findOne( 'div' ), range.endContainer, 'range.startContainer' );
+			assert.areSame( 0, range.startOffset, 'range.startOffset' );
+			assert.areSame( 1, range.endOffset, 'range.startOffset' );
 		}
 	} );
 } )();

tests/core/dom/range/extractcontents.js

@@ -46,7 +46,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( '<b>ome</b> t', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
+			assert.areSame( '<b id="_b">ome</b> t', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
 			assert.areSame( 'this is <b id="_b">s</b>ext.', getInnerHtml( '_Para' ), 'HTML after extraction' );
 
 			assert.areSame( document.getElementById( '_Para' ), range.startContainer.$, 'range.startContainer' );
@@ -68,7 +68,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( 'his is <b>s</b>', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
+			assert.areSame( 'his is <b id="_b">s</b>', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
 			assert.areSame( 't<b id="_b">ome</b> text.', getInnerHtml( '_Para' ), 'HTML after extraction' );
 
 			assert.areSame( document.getElementById( '_Para' ), range.startContainer.$, 'range.startContainer' );
@@ -90,7 +90,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( '<h1>ckw3crange test</h1><p id="_para">this is <b id="_b">some</b> text.</p><p>a</p>', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
+			assert.areSame( '<h1 id="_h1">ckw3crange test</h1><p id="_para">this is <b id="_b">some</b> text.</p><p>a</p>', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
 			assert.areSame( '<h1 id="_h1">f</h1><p>nother paragraph.</p>', getInnerHtml( 'playground' ) );
 
 			assert.areSame( document.getElementById( 'playground' ), range.startContainer.$, 'range.startContainer' );
@@ -110,7 +110,7 @@
 			var tmpDiv = doc.createElement( 'div' );
 			docFrag.appendTo( tmpDiv );
 
-			assert.areSame( '<h1>fckw3crange test</h1><p id="_para">this is <b id="_b">some</b> text.</p><p>another paragraph.</p>', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
+			assert.areSame( '<h1 id="_h1">fckw3crange test</h1><p id="_para">this is <b id="_b">some</b> text.</p><p>another paragraph.</p>', getInnerHtml( tmpDiv.$ ), 'Extracted HTML' );
 			assert.areSame( '<h1 id="_h1"></h1><p></p>', getInnerHtml( 'playground' ) );
 
 			assert.areSame( document.getElementById( 'playground' ), range.startContainer.$, 'range.startContainer' );
@@ -345,6 +345,28 @@
 
 			assert.isInnerHtmlMatching( '<h1></h1><h2></h2>', clone.getHtml() );
 			assert.isInnerHtmlMatching( '<h1></h1>[]<h2><br /></h2>', bender.tools.range.getWithHtml( root, range ) );
+		},
+
+		'test extractContents - ID attribute cloning of partially and fully selected elements': function() {
+			var range = new CKEDITOR.dom.range( doc );
+			range.setStart( doc.getById( '_H1' ).getFirst(), 11 );
+			range.setEnd( doc.getById( '_Para' ).getLast(), 2 );
+
+			var docFrag = range.extractContents( false, false );
+
+			var tmpDiv = doc.createElement( 'div' );
+			docFrag.appendTo( tmpDiv );
+
+			var playground = doc.getById( 'playground' );
+
+			// See: execContentsAction in range.js.
+			assert.isInnerHtmlMatching( '<h1>Test</h1><p>t<b id="_B">some</b>This is</p>', tmpDiv.getHtml(), 'Extracted HTML' );
+			assert.isInnerHtmlMatching( '<h1 id="_H1">FCKW3CRange</h1><p id="_Para">ext.</p><p>Another paragraph.</p>',
+				playground.getHtml(), 'HTML after extraction' );
+
+			assert.areSame( playground, range.startContainer, 'range.startContainer' );
+			assert.areSame( 1, range.startOffset, 'range.startOffset' );
+			assert.isTrue( range.collapsed, 'range.collapsed' );
 		}
 	};
 

tests/core/dom/range/split.html

@@ -0,0 +1,4 @@
+<div id="playground" style="visibility: hidden">
+	<p id="SE1">Test split <b id="SE1B1"><i id="SE1I1">element</i></b>.</p>
+</div>
+<div id="editable_playground" contenteditable="true"></div>