Ticket #3576: 3576.patch

core/tools.js

@@ -169 +169 @@
                 },
 
                 /**
+                 * Determine whether an given object has no own property.
+                 * @param {Object|null|undefined} object
+                 */
+                isEmpty : function( object )
+                {
+                        for ( var property in object )
+                                if ( object.hasOwnProperty( property ) )
+                                        return false;
+                        return true;
+                },
+
+                /**
                  * Transforms a CSS property name to its relative DOM style name.
                  * @param {String} cssName The CSS property name.
                  * @returns {String} The transformed name.

plugins/styles/plugin.js

@@ -81 +81 @@
         var objectElements      = { a:1,embed:1,hr:1,img:1,li:1,object:1,ol:1,table:1,td:1,tr:1,ul:1 };
 
         var semicolonFixRegex = /\s*(?:;\s*|$)/;
-
+        // Customizing CKEDITOR.dom.node::getNext for ignoring bookmark nodes.
+        function getNextSibling( currentNode )
+        {
+                var next = currentNode;
+                do
+                {
+                        next = next.getNext();
+                }
+                while( next && next.getName
+                                        && next.getName() == 'span'
+                                        && next.getAttribute( '_fck_bookmark' ) );
+                return next;
+        }
         CKEDITOR.style = function( styleDefinition, variablesValues )
         {
                 if ( variablesValues )
@@ -363 +375 @@
                                 var nodeType = currentNode.type;
                                 var nodeName = nodeType == CKEDITOR.NODE_ELEMENT ? currentNode.getName() : null;
 
-                                if ( nodeName && currentNode.getAttribute( '_fck_bookmark' ) )
+                                // Ignore bookmark nodes.
+                                if ( nodeName && currentNode.hasAttribute( '_fck_bookmark' )
+                                        || !nodeName && currentNode.getParent().hasAttribute( '_fck_bookmark' ) )
                                 {
                                         currentNode = currentNode.getNextSourceNode( true );
                                         continue;
@@ -399 +413 @@
                                                         // if this is the last node in its parent, we must also
                                                         // check if the parent itself can be added completelly
                                                         // to the range.
-                                                        while ( !includedNode.$.nextSibling
+                                                        while ( !getNextSibling( includedNode )
                                                                 && ( parentNode = includedNode.getParent(), dtd[ parentNode.getName() ] )
                                                                 && ( parentNode.getPosition( firstNode ) | CKEDITOR.POSITION_FOLLOWING | CKEDITOR.POSITION_IDENTICAL | CKEDITOR.POSITION_IS_CONTAINED ) == ( CKEDITOR.POSITION_FOLLOWING + CKEDITOR.POSITION_IDENTICAL + CKEDITOR.POSITION_IS_CONTAINED ) )
-                                                        {
                                                                 includedNode = parentNode;
-                                                        }
 
                                                         styleRange.setEndAfter( includedNode );
 
                                                         // If the included node still is the last node in its
                                                         // parent, it means that the parent can't be included
                                                         // in this style DTD, so apply the style immediately.
-                                                        if ( !includedNode.$.nextSibling )
+                                                        if ( !getNextSibling( includedNode ) )
                                                                 applyStyle = true;
 
                                                         if ( !hasContents )
@@ -675 +687 @@
         function removeFromElement( style, element )
         {
                 var def = style._.definition,
-                        attributes = def.attributes,
+                        attributes = CKEDITOR.tools.extend( {},
+                                        def.attributes,
+                                        // Override styles on the element.
+                                        getOverrides( style )[ element.getName() ] ),
                         styles = def.styles,
-                        overrides = getOverrides( style );
+                        // If the style is only about the element itself, we have to remove the element.
+                        removeEmpty = CKEDITOR.tools.isEmpty( attributes ) && CKEDITOR.tools.isEmpty( styles );
 
-                function removeAttrs()
+                //  If there's any to-be-removed style/attribute, remove them from the element,
+                //  then only remove the element if it's empty.
+                for ( var attName in attributes )
                 {
-                        for ( var attName in attributes )
-                        {
-                                // The 'class' element value must match (#1318).
-                                if ( attName == 'class' && element.getAttribute( attName ) != attributes[ attName ] )
-                                        continue;
-                                element.removeAttribute( attName );
-                        }
+                        // The 'class' element value must match (#1318).
+                        if ( attName == 'class' && element.getAttribute( attName ) != attributes[ attName ] )
+                                continue;
+                        element.removeAttribute( attName );
+                        removeEmpty = true;
                 }
 
-                // Remove definition attributes/style from the elemnt.
-                removeAttrs();
                 for ( var styleName in styles )
+                {
                         element.removeStyle( styleName );
-
-                // Now remove override styles on the element.
-                attributes = overrides[ element.getName() ];
-                if( attributes )
-                        removeAttrs();
-                removeNoAttribsElement( element );
+                        removeEmpty = true;
+                }
+                if ( removeEmpty )
+                        removeNoAttribsElement( element );
         }
 
         // Removes a style from inside an element.

tests/plugins/styles/styles.html

@@ -206 +206 @@
                                 } );
                         style.applyToRange( range );
 
-                        assert.areSame( '<b lang="it" style="font-size: 10pt; text-decoration: line-through;" title="test">this <b class="sample">is</b> some sample text</b>', getInnerHtml( '_P1' ) );
+                        assert.areSame( '<b lang="it" style="font-size:10pt;text-decoration:line-through;" title="test">this <b class="sample">is</b> some sample text</b>', getInnerHtml( '_P1' ) );
                 },
 
-                test_inline11 : function()
+                test_inline12 : function()
                 {
                         doc.getById( '_P1' ).setHtml( 'this <span class="a">is</span> some <span class="b">sample</span> text' );
 
@@ -223 +223 @@
                         assert.areSame( '<span class="b">this <span class="a">is</span> some sample text</span>', getInnerHtml( '_P1' ) );
                 },
 
-                test_inline12 : function()
+                test_inline13 : function()
                 {
-                        doc.getById( '_P1' ).setHtml( 'this <span style="font-size:12pt; font-weight:600">is</span> some <span style="font-size:10px;">sample</span> text' );
+                        doc.getById( '_P1' ).setHtml( 'this <span style="font-size:12pt;font-weight:600">is</span> some <span style="font-size:10px;">sample</span> text' );
 
                         var range = new CKEDITOR.dom.range( doc );
                         range.setStart( doc.getById( '_P1' ), 0 );
@@ -237 +237 @@
                         assert.areSame( '<span style="font-size:1.5em;">this <span style="font-weight:600;">is</span> some sample text</span>', getInnerHtml( '_P1' ) );
                 },
 
-                test_inline13 : function()
+                test_inline14 : function()
                 {
                         doc.getById( '_P1' ).setHtml( 'this <b>is some sample</b> text' );
 
@@ -251 +251 @@
                         assert.areSame( 'this <b>is <i>some sample</i></b><i> text</i>', getInnerHtml( '_P1' ) );
                 },
 
-                test_inline14 : function()
+                test_inline15 : function()
                 {
                         var para = doc.getById( '_P1' );
 
@@ -277 +277 @@
                         assert.areSame( '<b>this is some</b> sample text', getInnerHtml( '_P1' ), 'Second range' );
                 },
 
-                test_inline15 : function()
+                test_inline16 : function()
                 {
                         var para = doc.getById( '_P1' );
 
@@ -304 +304 @@
                         assert.areSame( '<span style="font-family:arial,helvetica,sans-serif;">this <span style="font-family:georgia,serif;">is</span></span><span style="font-family:georgia,serif;"> some</span> sample text', getInnerHtml( '_P1' ), 'Second range' );
                 },
 
-                test_inline16 : function()
+                test_inline17 : function()
                 {
                         var para = doc.getById( '_P1' );
 
@@ -336 +336 @@
 
                 test_ticket_2040 : function()
                 {
-                        doc.getById( '_P1' ).setHtml( 'This is some <strong>sample text<\/strong>. You are using <a href="http://www.fckeditor.net/">CKEditor<\/a>.' );
+                        doc.getById( '_P1' ).setHtml( 'This is some <strong>sample text<\/strong>. You are using <a href="http://www.fckeditor.net/">ckeditor<\/a>.' );
 
                         var range = new CKEDITOR.dom.range( doc );
                         range.setStart( doc.getById( '_P1' ), 1 );
@@ -345 +345 @@
                         var style = new CKEDITOR.style( { element : 'i' } );
                         style.applyToRange( range );
 
-                        assert.areSame( 'this is some <strong><i>sample</i> text<\/strong>. you are using <a href="http://www.fckeditor.net/">CKEditor<\/a>.', getInnerHtml( '_P1' ) );
+                        assert.areSame( 'this is some <strong><i>sample</i> text<\/strong>. you are using <a href="http://www.fckeditor.net/">ckeditor<\/a>.', getInnerHtml( '_P1' ) );
                 },
 
                 test_checkElementRemovable1 : function()
@@ -486 +486 @@
                         assert.areSame( '<p><i title="z">text</i></p><i title="z">outter</i>', getInnerHtml( element ) );
                 },
 
+                test_ticket_3576 : function()
+                {
+                        var element = doc.getById( '_P1' );
+                        element.setHtml( '<span>inline</span>' );
+
+                        var range = new CKEDITOR.dom.range( doc );
+                        range.selectNodeContents( element.getFirst().getFirst() );
+
+                        var style = new CKEDITOR.style( { element : 'span', attributes : { class : 'style' } } );
+                        style.applyToRange( range );
+
+                        assert.areSame( '<span class="style"><span>inline</span></span>', getInnerHtml( element ) );
+                },
+
                 name : document.title
         };
 })() );
-
+//window.onload = testCase.test_inline11;
         //]]>
         </script>
 </head>