Index: /CKEditor/trunk/_source/core/dom/range.js =================================================================== --- /CKEditor/trunk/_source/core/dom/range.js (revision 3689) +++ /CKEditor/trunk/_source/core/dom/range.js (revision 3690) @@ -296,4 +296,14 @@ }; } + + // Evaluator for CKEDITOR.dom.element::checkBoundaryOfElement, reject any + // text node and non-empty elements unless it's being bookmark text. + function elementBoundaryEval( node ) + { + // Reject any text node unless it's being bookmark. + return node.type != CKEDITOR.NODE_TEXT + && node.getName() in CKEDITOR.dtd.$removeEmpty + || node.getParent().hasAttribute( '_fck_bookmark' ); + }; CKEDITOR.dom.range.prototype = @@ -1468,4 +1478,25 @@ }, + /** + * Check whether current range is on the inner edge of the specified element. + * @param {Number} checkType ( CKEDITOR.START | CKEDITOR.END ) The checking side. + * @param {CKEDITOR.dom.element} element The target element to check. + */ + checkBoundaryOfElement : function( element, checkType ) + { + var walkerRange = this.clone(); + // Expand the range to element boundary. + walkerRange[ checkType == CKEDITOR.START ? + 'setStartAt' : 'setEndAt' ] + ( element, checkType == CKEDITOR.START ? + CKEDITOR.POSITION_AFTER_START + : CKEDITOR.POSITION_BEFORE_END ); + + var walker = new CKEDITOR.dom.walker( walkerRange ), + retval = false; + walker.evaluator = elementBoundaryEval; + return walker[ checkType == CKEDITOR.START ? + 'checkBackward' : 'checkForward' ](); + }, // Calls to this function may produce changes to the DOM. The range may // be updated to reflect such changes. @@ -1594,2 +1625,10 @@ CKEDITOR.ENLARGE_BLOCK_CONTENTS = 2; CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS = 3; + +/** + * Check boundary types. + * @see CKEDITOR.dom.range::checkBoundaryOfElement + */ +CKEDITOR.START = 1; +CKEDITOR.END = 2; +CKEDITOR.STARTEND = 3; Index: /CKEditor/trunk/_source/plugins/styles/plugin.js =================================================================== --- /CKEditor/trunk/_source/plugins/styles/plugin.js (revision 3689) +++ /CKEditor/trunk/_source/plugins/styles/plugin.js (revision 3690) @@ -1,3 +1,3 @@ -/* +/* Copyright (c) 2003-2009, CKSource - Frederico Knabben. All rights reserved. For licensing, see LICENSE.html or http://ckeditor.com/license @@ -511,16 +511,25 @@ 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 make sure other inner styles were well preserverd.(#3309) + */ if ( element == startPath.block || element == startPath.blockLimit ) break; @@ -528,13 +537,47 @@ 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 ); - } + var endOfElement = range.checkBoundaryOfElement( element, CKEDITOR.END ), + startOfElement = !endOfElement && range.checkBoundaryOfElement( element, CKEDITOR.START ); + if ( startOfElement || endOfElement ) + { + boundaryElement = element; + boundaryElement.match = startOfElement ? 'start' : 'end'; + } + 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 ); + + } + } + } + + // Re-create the style tree after/before the boundary element, + // the replication start from bookmark start node to define the + // new range. + if ( boundaryElement ) + { + var clonedElement = startNode; + for ( var i = 0 ;; i++ ) + { + var newElement = startPath.elements[ i ]; + if ( newElement.equals( boundaryElement ) ) + break; + // Avoid copying any matched element. + else if( newElement.match ) + continue; + else + newElement = newElement.clone(); + newElement.append( clonedElement ); + clonedElement = newElement; + } + clonedElement[ boundaryElement.match == 'start' ? + 'insertBefore' : 'insertAfter' ]( boundaryElement ); } } Index: /CKEditor/trunk/_source/tests/plugins/styles/styles.html =================================================================== --- /CKEditor/trunk/_source/tests/plugins/styles/styles.html (revision 3689) +++ /CKEditor/trunk/_source/tests/plugins/styles/styles.html (revision 3690) @@ -337,5 +337,5 @@ test_ticket_2040 : function() { - doc.getById( '_P1' ).setHtml( 'This is some sample text<\/strong>. You are using CKEditor<\/a>.' ); + doc.getById( '_P1' ).setHtml( 'This is some sample text<\/strong>. You are using ckeditor<\/a>.' ); var range = new CKEDITOR.dom.range( doc ); @@ -346,5 +346,5 @@ style.applyToRange( range ); - assert.areSame( 'this is some sample text<\/strong>. you are using CKEditor<\/a>.', getInnerHtml( '_P1' ) ); + assert.areSame( 'this is some sample text<\/strong>. you are using ckeditor<\/a>.', getInnerHtml( '_P1' ) ); }, @@ -487,8 +487,59 @@ }, + // Remove inline style when range collapsed at element boundaries, + // move out of the removing-style element, with inner style copied. + test_ticket_3309 : function() + { + var element = doc.getById( '_P1' ); + element.setHtml( 'this is some styles text' ); + + // This is some styles^ text + var range = new CKEDITOR.dom.range( doc ); + range.setStartAt( doc.getById( '_i1' ), CKEDITOR.POSITION_BEFORE_END ); + + var style = new CKEDITOR.style( { element : 'b' } ); + style.removeFromRange( range ); + + assert.areSame( 'this is some styles text', getInnerHtml( element ) ); + }, + + // No inner style preserved, simply move out of the removing-style element. + test_ticket_3309_2 : function() + { + var element = doc.getById( '_P1' ); + element.setHtml( 'this is some styles text' ); + + // This is some styles^ text + var range = new CKEDITOR.dom.range( doc ); + range.setStartAt( doc.getById( '_b1' ), CKEDITOR.POSITION_BEFORE_END ); + + var style = new CKEDITOR.style( { element : 'b' } ); + style.removeFromRange( range ); + // This is some styles^ text + assert.areSame( doc.getById( '_b1' ).getParent().$, range.startContainer.$ ); + assert.areSame( 2, range.startOffset ); + assert.areSame( 'this is some styles text', getInnerHtml( element ) ); + }, + + // With style overrides. + test_ticket_3309_3 : function() + { + var element = doc.getById( '_P1' ); + element.setHtml( 'text styles' ); + + // text ^styles + var range = new CKEDITOR.dom.range( doc ); + range.setStartAt( doc.getById( '_i1' ), CKEDITOR.POSITION_AFTER_START ); + + var style = new CKEDITOR.style( { element : 'b' , overrides : [ 'strong', 'bold' ] } ); + style.removeFromRange( range ); + + // text ^styles + assert.areSame( 'text styles', getInnerHtml( element ) ); + }, name : document.title }; })() ); - +//window.onload = testCase.test_ticket_3309_3; //]]>