From db8ada67b81832e00e6bf226796a89aa7d7db99e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Reinmar=20Koszuli=C5=84ski?= Date: Fri, 22 Nov 2013 11:31:03 +0100 Subject: [PATCH 1/4] Fix range placed in intermediate position inside table. --- core/editable.js | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/core/editable.js b/core/editable.js index 81feb7a51f3..e37677e41e4 100644 --- a/core/editable.js +++ b/core/editable.js @@ -267,6 +267,9 @@ // the parent blocks until we reach blockLimit. var current, dtd; + if ( range.startContainer.type == CKEDITOR.NODE_ELEMENT && range.startContainer.is( { tr:1,table:1,tbody:1,thead:1,tfoot:1 } ) ) + fixTableSelectionForInsertion( range ); + if ( isBlock ) { while ( ( current = range.getCommonAncestor( 0, 1 ) ) && ( dtd = CKEDITOR.dtd[ current.getName() ] ) && @@ -1810,6 +1813,80 @@ }, 0 ); } + // Fixes a range which is a result of deleteContents() and is placed in an intermediate element (see dtd.$intermediate), + // inside a table. A goal is to find a closest or element and when this fails, recreate the structure of the table. + var fixTableSelectionForInsertion = (function() { + // Creates an element walker which can only "go deeper". It won't + // move out from any element. Therefore it can be used to find x in cases like: + // ^... + function getFixTableSelectionWalker( testRange ) { + var walker = new CKEDITOR.dom.walker( testRange ); + walker.guard = function( node, isMovingOut ) { + if ( isMovingOut ) + return false; + if ( node.type == CKEDITOR.NODE_ELEMENT ) + return node.is( CKEDITOR.dtd.$tableContent ); + }; + walker.evaluator = function( node ) { + return node.type == CKEDITOR.NODE_ELEMENT; + }; + + return walker; + } + + function fixTableStructure( element, newElementName, appendToStart ) { + var temp = element.getDocument().createElement( newElementName ); + element.append( temp, appendToStart ); + return temp; + } + + return function( range ) { + var container = range.startContainer, + testRange, + walker, + deeperSibling, + doc = range.document, + appendToStart = false; + + // Look left. + testRange = range.clone(); + testRange.setStart( container, 0 ); + deeperSibling = getFixTableSelectionWalker( testRange ).lastBackward(); + + // If left is empty, look right. + if ( !deeperSibling ) { + testRange = range.clone(); + testRange.setEndAt( container, CKEDITOR.POSITION_BEFORE_END ); + deeperSibling = getFixTableSelectionWalker( testRange ).lastForward(); + appendToStart = true; + } + + // If there's no deeper nested element in both direction - container is empty - we'll use it then. + if ( !deeperSibling ) + deeperSibling = container; + + // Fix structure... + + // We found a table what means that it's empty - remove it completely. + if ( deeperSibling.is( 'table' ) ) { + range.setStartAt( deeperSibling, CKEDITOR.POSITION_BEFORE_START ); + range.collapse( true ); + deeperSibling.remove(); + return; + } + + // Found an empty txxx element - append tr. + if ( deeperSibling.is( { tbody:1,thead:1,tfoot:1 } ) ) + deeperSibling = fixTableStructure( deeperSibling, 'tr', appendToStart ); + + // Found an empty tr element - append td/th. + if ( deeperSibling.is( 'tr' ) ) + deeperSibling = fixTableStructure( deeperSibling, deeperSibling.getParent().is( 'thead' ) ? 'th' : 'td', appendToStart ); + + range.moveToPosition( deeperSibling, appendToStart ? CKEDITOR.POSITION_AFTER_START : CKEDITOR.POSITION_BEFORE_END ); + } + })(); + })(); /** From b8edfaa4f39e8bd0fc5dd92a41090f5a807dfb52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Reinmar=20Koszuli=C5=84ski?= Date: Tue, 26 Nov 2013 15:01:31 +0100 Subject: [PATCH 2/4] Prevent from IE8 crash and fill empty cells with bogus brs. --- core/editable.js | 43 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/core/editable.js b/core/editable.js index e37677e41e4..01ebb2f6f1f 100644 --- a/core/editable.js +++ b/core/editable.js @@ -263,13 +263,17 @@ // Remove the original contents, merge split nodes. range.deleteContents( 1 ); + // If range is placed in inermediate element (not td or th), we need to do three things: + // * fill emptied in cases like: //
x
s with if browser needs them, + // * remove empty text nodes so IE8 won't crash (http://dev.ckeditor.com/ticket/11183#comment:8), + // * fix structure and move range into the element. + if ( range.startContainer.type == CKEDITOR.NODE_ELEMENT && range.startContainer.is( { tr:1,table:1,tbody:1,thead:1,tfoot:1 } ) ) + fixTableAfterContentsDeletion( range ); + // If we're inserting a block at dtd-violated position, split // the parent blocks until we reach blockLimit. var current, dtd; - if ( range.startContainer.type == CKEDITOR.NODE_ELEMENT && range.startContainer.is( { tr:1,table:1,tbody:1,thead:1,tfoot:1 } ) ) - fixTableSelectionForInsertion( range ); - if ( isBlock ) { while ( ( current = range.getCommonAncestor( 0, 1 ) ) && ( dtd = CKEDITOR.dtd[ current.getName() ] ) && @@ -1813,9 +1817,10 @@ }, 0 ); } - // Fixes a range which is a result of deleteContents() and is placed in an intermediate element (see dtd.$intermediate), + // 1. Fixes a range which is a result of deleteContents() and is placed in an intermediate element (see dtd.$intermediate), // inside a table. A goal is to find a closest or element and when this fails, recreate the structure of the table. - var fixTableSelectionForInsertion = (function() { + // 2. Fixes empty cells by appending bogus
s or deleting empty text nodes in IE<=8 case. + var fixTableAfterContentsDeletion = (function() { // Creates an element walker which can only "go deeper". It won't // move out from any element. Therefore it can be used to find 		x
^... @@ -1840,14 +1845,36 @@ return temp; } + // Fix empty cells. This means: + // * add bogus
if browser needs it + // * remove empty text nodes on IE8, because it will crash (http://dev.ckeditor.com/ticket/11183#comment:8). + function fixEmptyCells( cells ) { + var i = cells.count(), + cell; + + for ( i; i-- > 0; ) { + cell = cells.getItem( i ); + + if ( !CKEDITOR.tools.trim( cell.getHtml() ) ) { + cell.appendBogus(); + if ( CKEDITOR.env.ie && CKEDITOR.env.version < 9 && cell.getChildCount() ) + cell.getFirst().remove(); + } + } + } + return function( range ) { var container = range.startContainer, + table = container.getAscendant( 'table', 1 ), testRange, walker, deeperSibling, doc = range.document, appendToStart = false; + fixEmptyCells( table.getElementsByTag( 'td' ) ); + fixEmptyCells( table.getElementsByTag( 'th' ) ); + // Look left. testRange = range.clone(); testRange.setStart( container, 0 ); @@ -1883,6 +1910,12 @@ if ( deeperSibling.is( 'tr' ) ) deeperSibling = fixTableStructure( deeperSibling, deeperSibling.getParent().is( 'thead' ) ? 'th' : 'td', appendToStart ); + // To avoid setting selection after bogus, remove it from the current cell. + // We can safely do that, because we'll insert element into that cell. + var bogus = deeperSibling.getBogus(); + if ( bogus ) + bogus.remove(); + range.moveToPosition( deeperSibling, appendToStart ? CKEDITOR.POSITION_AFTER_START : CKEDITOR.POSITION_BEFORE_END ); } })(); From f1b07b325511a4c0ad769a33800f11397811b125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Reinmar=20Koszuli=C5=84ski?= Date: Tue, 26 Nov 2013 15:52:27 +0100 Subject: [PATCH 3/4] Insert element into the first range only in case of multirange selection. --- core/editable.js | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/core/editable.js b/core/editable.js index 01ebb2f6f1f..9e00458f220 100644 --- a/core/editable.js +++ b/core/editable.js @@ -310,48 +310,32 @@ var editor = this.editor, enterMode = editor.activeEnterMode, selection = editor.getSelection(), - ranges = selection.getRanges(), + range = selection.getRanges()[ 0 ], elementName = element.getName(), - isBlock = CKEDITOR.dtd.$block[ elementName ], - clone, lastElement, range; + isBlock = CKEDITOR.dtd.$block[ elementName ]; // Prepare for the insertion. beforeInsert( this ); - // Insert the element into all ranges by cloning. - for ( var i = ranges.length; i--; ) { - range = ranges[ i ]; - - // Clone is an element for the first range. - clone = !i && element || element.clone( 1 ); - - // Put the clone into a particular range. - // Save the last **successfully inserted** element reference - // so we can make the selection later. - if ( this.insertElementIntoRange( clone, range ) && !lastElement ) - lastElement = clone; - } - - if ( lastElement ) { - range.moveToPosition( lastElement, CKEDITOR.POSITION_AFTER_END ); + // Insert element into first range only and ignore the rest (#11183). + if ( this.insertElementIntoRange( element, range ) ) { + range.moveToPosition( element, CKEDITOR.POSITION_AFTER_END ); // If we're inserting a block element, the new cursor position must be // optimized. (#3100,#5436,#8950) if ( isBlock ) { - // Find next, meaningful element. - var next = lastElement.getNext( function( node ) { + var next = element.getNext( function( node ) { return isNotEmpty( node ) && !isBogus( node ); } ); if ( next && next.type == CKEDITOR.NODE_ELEMENT && next.is( CKEDITOR.dtd.$block ) ) { - // If the next one is a text block, move cursor to the start of it's content. if ( next.getDtd()[ '#' ] ) range.moveToElementEditStart( next ); // Otherwise move cursor to the before end of the last element. else - range.moveToElementEditEnd( lastElement ); + range.moveToElementEditEnd( element ); } // Open a new line if the block is inserted at the end of parent. else if ( !next && enterMode != CKEDITOR.ENTER_BR ) { From f966e67ff53334592f4dd3629ea1b21c5ba08d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Reinmar=20Koszuli=C5=84ski?= Date: Tue, 3 Dec 2013 11:47:37 +0100 Subject: [PATCH 4/4] Changelog entry. --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 5a23b4a3535..348aa9725d5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,7 @@ Fixed Issues: * [#11202](http://dev.ckeditor.com/ticket/11202): Fixed: No newline at BB-code mode. * [#10890](http://dev.ckeditor.com/ticket/10890): Fixed: Error thrown when pressing *Delete* key in a list item. * [#10055](http://dev.ckeditor.com/ticket/10055): [IE8-10] Fixed: *Delete* pressed on selected image causes browser to go back. +* [#11183](http://dev.ckeditor.com/ticket/11183): Fixed: Inserting line or table in multiple rows selection causes browser crash. Additionally, the [`editor.insertElement`](http://docs.ckeditor.com/#!/api/CKEDITOR.editor-method-insertElement) method does not insert element into every range of a selection any more. ## CKEditor 4.3
x