Ticket #3309: 3309_2.patch

_source/core/dom/documentfragment.js

@@ -43 +43 @@
                 'getDocument' : 1,
                 'getChildCount' : 1,
                 'getChild' : 1,
-                'getChildren' : 1
+                'getChildren' : 1,
+                'type' : 1
         } );

_source/plugins/styles/plugin.js

@@ -510 +510 @@
                  */
                 range.enlarge( CKEDITOR.ENLARGE_ELEMENT );
 
-                var bookmark = range.createBookmark( true ),
-                        startNode = range.document.getById( bookmark.startNode );
+                var bookmark = range.createBookmark(),
+                        startNode = bookmark.startNode;
 
                 if ( range.collapsed )
                 {
-                        /*
-                         * If the range is collapsed, try to remove the style from all ancestor
-                         * elements, until a block boundary is reached.
-                         */
-                        var startPath = new CKEDITOR.dom.elementPath( startNode.getParent() );
-                        for ( var i = 0, element ; i < startPath.elements.length && ( element = startPath.elements[i] ) ; i++ )
-                        {
+                        
+                        var startPath = new CKEDITOR.dom.elementPath( startNode.getParent() ),
+                                // The topmost element in elementspatch which we should jump out of.
+                                boundaryElement;
+
+
+                        for ( var i = 0, element ; i < startPath.elements.length
+                                        && ( element = startPath.elements[i] ) ; i++ )
+                        {
+                                /*
+                                 * 1. If it's collaped inside text nodes, try to remove the style from the whole element.
+                                 *
+                                 * 2. Otherwise if it's collapsed on element boundaries, moving the selection
+                                 *  outside the styles instead of removing the whole tag,
+                                 *  also makre sure other inner styles were well preserverd.(#3309)
+                                 */
                                 if ( element == startPath.block || element == startPath.blockLimit )
                                         break;
 
                                 if ( this.checkElementRemovable( element ) )
                                 {
-                                        /*
-                                         * Before removing the style node, there may be a sibling to the style node
-                                         * that's exactly the same to the one to be removed. To the user, it makes
-                                         * no difference that they're separate entities in the DOM tree. So, merge
-                                         * them before removal.
-                                         */
-                                        mergeSiblings( element );
-                                        removeFromElement( this, element );
-                                }
-                        }
-                }
-                else
+                                        var startOfElement = range.checkBoundaryOfElement( element, CKEDITOR.START_OF_ELEMENT ),
+                                                endOfElement = range.checkBoundaryOfElement( element, CKEDITOR.END_OF_ELEMENT );
+                                        if ( startOfElement || endOfElement )
+                                        {
+                                                boundaryElement = element;
+                                                boundaryElement.isStart = startOfElement;
+                                        }
+                                        else
+                                        {
+                                                /*
+                                                 * Before removing the style node, there may be a sibling to the style node
+                                                 * that's exactly the same to the one to be removed. To the user, it makes
+                                                 * no difference that they're separate entities in the DOM tree. So, merge
+                                                 * them before removal.
+                                                 */
+                                                mergeSiblings( element );
+                                                removeFromElement( this, element );
+
+                                        }
+                                }
+                        }
+
+                        if ( boundaryElement )
+                        {
+                                // Range is guaranteed to be collapsed inside the inner-most inline element,
+                                // so simply start from a cloned range.
+                                var newStyleRange = range.clone();
+
+                                // Include it in the new range.
+                                newStyleRange[ boundaryElement.isStart ?
+                                  'setStartBefore' : 'setEndAfter'].call( newStyleRange, boundaryElement );
+
+                                var styleElements = newStyleRange.cloneContents();
+
+                                // We need build a temp node here for 'removeFromInsideElement'.
+                                var temp = range.document.createElement( 'div' );
+                                temp.append( styleElements );
+                                removeFromInsideElement( this, temp );
+
+                                // It's guaranteed that style elements is single rooted.
+                                styleElements = temp.getFirst();
+
+                                // Start from the new position right before/after the topmost target style element.
+                                newStyleRange.collapse( boundaryElement.isStart ? true : false );
+
+                                // Continue with the inner styles.
+                                if ( styleElements )
+                                {
+                                        newStyleRange.insertNode( styleElements );
+                                        mergeSiblings( styleElements );
+                                        // Destroy the bookmark node.
+                                        startNode.remove();
+                                        range.moveToElementEditStart( styleElements );
+                                        return;
+                                }
+                                else
+                                        // No inner styles found, simply jump out of boundary element.
+                                        newStyleRange.insertNode( bookmark.startNode );
+                        }
+                }
+                else
                 {
                         /*
                          * Now our range isn't collapsed. Lets walk from the start node to the end

_source/core/dom/range.js

@@ -135 +135 @@
 
                         while( currentNode )
                         {
+                                // Avoid cloning bookmark nodes.
+                                if ( action == 2 && 
+                                     currentNode.hasAttribute && currentNode.hasAttribute( '_fck_bookmark' ) )
+                                {
+                                        currentNode = currentSibling;
+                                        continue;
+                                }
                                 // Stop processing when the current node matches a node in the
                                 // endParents tree or if it is the endNode.
                                 if ( currentNode.equals( endParents[ j ] ) || currentNode.equals( endNode ) )
@@ -182 +189 @@
 
                                 while( currentNode )
                                 {
+                                        // Avoid cloning bookmark nodes.
+                                        if ( action == 2 &&
+                                             currentNode.hasAttribute && currentNode.hasAttribute( '_fck_bookmark' ) )
+                                        {
+                                                currentNode = currentSibling;
+                                                continue;
+                                        }
                                         // Stop processing when the current node matches a node in the
                                         // startParents tree or if it is the startNode.
                                         if ( currentNode.equals( startParents[ k ] ) || currentNode.equals( startNode ) )
@@ -292 +306 @@
                 };
         }
 
+        // Evaluator for CKEDITOR.dom.element::checkBoundaryOfElement, reject all text type node,
+        // while ignore bookmark node inner text.
+        function elementBoundaryEval( node )
+        {
+                // Reject any text node unless it's being bookmark.
+                return node.type != CKEDITOR.NODE_TEXT || node.getParent().hasAttribute( '_fck_bookmark' );
+        };
+
         CKEDITOR.dom.range.prototype =
         {
                 clone : function()
@@ -1427 +1449 @@
                         };
                 },
 
+                /**
+                 * Whether range is on the boundary of an element node.
+                 * @param {Number} checkType ( CKEDITOR.START_OF_ELEMENT|CKEDITOR.END_OF_ELEMENT)
+                 *  Specify check the range is at the start OR end of the element.
+                 */
+                checkBoundaryOfElement : function( element, checkType )
+                {
+                        var matched = false;
+                        var walkerRange = this.clone();
+                        // Expand the range clockwise.
+                        walkerRange[ checkType == CKEDITOR.START_OF_ELEMENT ?
+                                'setStartBefore' : 'setEndAfter'].call(
+                                        walkerRange, this.document.getBody() );
+                        var walker = new CKEDITOR.dom.walker( walkerRange ),
+                                retval = false;
+                        walker.evaluator = elementBoundaryEval;
+                        // Try to match among all the found boundary elements.
+                        walker.guard = function( node )
+                        {
+                                if ( node.type == CKEDITOR.NODE_ELEMENT && element.equals( node ) )
+                                        matched = true;
+                        };
+
+                        walker[ checkType == CKEDITOR.START_OF_ELEMENT ?
+                                'checkBackward' : 'checkForward' ].call( walker );
+                        return matched;
+                },
+
                 // Calls to this function may produce changes to the DOM. The range may
                 // be updated to reflect such changes.
                 checkStartOfBlock : function()
@@ -1553 +1603 @@
 CKEDITOR.ENLARGE_ELEMENT = 1;
 CKEDITOR.ENLARGE_BLOCK_CONTENTS = 2;
 CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS = 3;
+
+/**
+ * Check boundary types.
+ * @see CKEDITOR.dom.range::checkBoundaryOfElement
+ */
+CKEDITOR.START_OF_ELEMENT = 1;
+CKEDITOR.END_OF_ELEMENT = 2;
+CKEDITOR.STARTEND_OF_ELEMENT = 3;