Merge branch 't/13568'

Author: Reinmar

CHANGES.md

@@ -10,6 +10,7 @@ New Features:
 Fixed Issues:
 
 * [#13386](http://dev.ckeditor.com/ticket/13386): [Edge] Fixed: Issues with selecting and editing images.
+* [#13568](http://dev.ckeditor.com/ticket/13568): Fixed: Method [`editor.getSelectedHtml()`](http://docs.ckeditor.com/#!/api/CKEDITOR.editor-method-getSelectedHtml) returns invalid results for entire content selection.
 
 ## CKEditor 4.5.2
 

core/dom/range.js

@@ -310,13 +310,6 @@ CKEDITOR.dom.range = function( root ) {
 			// The second step is to handle full selection of the content between the left branch and the right branch.
 
 			while ( nextSibling ) {
-				// TODO this case and the below are exactly the same because endNode is part of endParents.
-				// The start and end nodes are on the same level. Handle this case fully here, because it's simple.
-				if ( nextSibling.equals( endNode ) ) {
-					lastConnectedLevel = level;
-					break;
-				}
-
 				// We can't clone entire endParent just like we can't clone entire startParent -
 				// - they are not fully selected with the range. Partial endParent selection
 				// will be cloned in the next loop.
@@ -374,10 +367,18 @@ CKEDITOR.dom.range = function( root ) {
 				}
 
 				levelParent = nextLevelParent;
+			} else if ( doClone ) {
+				// If this is "shared" node and we are in cloning mode we have to update levelParent to
+				// reflect that we visited the node (even though we didn't process it).
+				// If we don't do that, in next iterations nodes will be appended to wrong parent.
+				//
+				// We can just take first child because the algorithm guarantees
+				// that this will be the only child on this level. (#13568)
+				levelParent = levelParent.getChild( 0 );
 			}
 		}
 
-		// Detete or Extract.
+		// Delete or Extract.
 		// We need to update the range and if mergeThen was passed do it.
 		if ( !isClone ) {
 			mergeAndUpdate();

tests/core/dom/range/clonecontents.html

@@ -1,2 +1,3 @@
 <div id="playground" style="visibility:hidden"><h1 id="_H1">FCKW3CRange Test</h1><p id="_Para">This is <b id="_B">some</b> text.</p><p>Another paragraph.</p></div>
-<div id="editable_playground" contenteditable="true"></div>
+<div id="editable_playground" contenteditable="true"></div>
+<div id="bogus"><p>Foo bar<br /></p></div>

tests/core/dom/range/clonecontents.js

@@ -514,6 +514,18 @@
 			assert.areSame( root.findOne( 'div' ), range.endContainer, 'range.startContainer' );
 			assert.areSame( 0, range.startOffset, 'range.startOffset' );
 			assert.areSame( 1, range.endOffset, 'range.startOffset' );
+		},
+
+		// #13568.
+		'test cloneContents - bogus br': function() {
+			var range = new CKEDITOR.dom.range( doc );
+			range.setStart( doc.getById( 'bogus' ), 0 ); // <p>
+			range.setEnd( doc.getById( 'bogus' ).getFirst(), 1 ); // <br /> in <p>
+
+			var docFrag = range.cloneContents();
+
+			// See: execContentsAction in range.js.
+			assert.isInnerHtmlMatching( '<p>Foo bar</p>', docFrag.getHtml(), 'Cloned HTML' );
 		}
 	} );
-} )();
+} )();

tests/core/dom/range/extractcontents.html

@@ -1 +1,2 @@
-<div id="playground" style="visibility:hidden"><h1 id="_H1">FCKW3CRange Test</h1><p id="_Para">This is <b id="_B">some</b> text.</p><p>Another paragraph.</p></div>
+<div id="playground" style="visibility:hidden"><h1 id="_H1">FCKW3CRange Test</h1><p id="_Para">This is <b id="_B">some</b> text.</p><p>Another paragraph.</p></div>
+<div id="bogus"><p>Foo bar<br /></p></div>

tests/core/dom/range/extractcontents.js

@@ -367,8 +367,20 @@
 			assert.areSame( playground, range.startContainer, 'range.startContainer' );
 			assert.areSame( 1, range.startOffset, 'range.startOffset' );
 			assert.isTrue( range.collapsed, 'range.collapsed' );
+		},
+
+		// #13568.
+		'test extractContents - bogus br': function() {
+			var range = new CKEDITOR.dom.range( doc );
+			range.setStart( doc.getById( 'bogus' ), 0 ); // <p>
+			range.setEnd( doc.getById( 'bogus' ).getFirst(), 1 ); // <br /> in <p>
+
+			var docFrag = range.extractContents();
+
+			// See: execContentsAction in range.js.
+			assert.isInnerHtmlMatching( '<p>Foo bar</p>', docFrag.getHtml(), 'Extracted HTML' );
 		}
 	};
 
 	bender.test( tests );
-} )();
+} )();

tests/core/editable/getextracthtmlfromrange.js

@@ -198,7 +198,10 @@
 			[ '<p><b>{a}</b>@</p>',													'<b>a</b>',														'<p>[]@!</p>' ],
 			[ '<p>{a}<br />@</p>',													'a',															'<p>[]<br />@</p>' ],
 			[ '<p>{a<br />]@</p>',													'a<br />',														'<p>[]@!</p>' ],
-			[ '<div>b<p>{a@]</p>b</div>',											'a',															'<div>b<p>[]@!</p>b</div>' ]
+			[ '<div>b<p>{a@]</p>b</div>',											'a',															'<div>b<p>[]@!</p>b</div>' ],
+			// #13568.
+			[ '<div>[<p>Foo bar@</p>]</div>',										'<p>Foo bar</p>',												'<div>[]@!</div>' ]
+
 		],
 
 		// #13101

tests/core/editor/getextractselectedhtml.js

@@ -78,6 +78,15 @@ bender.test( {
 		assert.isNull( selectedHtml, 'There should be no error but null should be returns if selection contains no ranges' );
 	},
 
+	// #13568.
+	'test getSelectedHtml with possible bogus br': function() {
+		var editor = this.editors.editor,
+			filler = CKEDITOR.env.needsBrFiller ? '<br />' : '';
+		bender.tools.selection.setWithHtml( editor, '[<p>Foo bar' + filler + '</p>]' );
+
+		assert.isInnerHtmlMatching( [ '<p>Foo bar@</p>', 'Foo bar' ], editor.getSelectedHtml( true ) );
+	},
+
 	'test extractSelectedHtml': function() {
 		var editor = this.editors.editor;
 		bender.tools.selection.setWithHtml( editor, '<p>fo{ob}ar</p>' );