Ticket #3195: 3195.patch

_source/core/htmlparser/fragment.js

@@ -41 +41 @@
         // (expect empty elements).
         var optionalClose = {colgroup:1,dd:1,dt:1,li:1,option:1,p:1,td:1,tfoot:1,th:1,thead:1,tr:1};
 
+        var tableBlocks = {table:1,tbody:1,thead:1,tfoot:1,tr:1};
+
         /**
          * Creates a {@link CKEDITOR.htmlParser.fragment} from an HTML string.
          * @param {String} fragmentHtml The HTML to be parsed, filling the fragment.
@@ -56 +58 @@
                         html = [],
                         fragment = new CKEDITOR.htmlParser.fragment(),
                         pendingInline = [],
-                        currentNode = fragment;
+                        currentNode = fragment,
+                        returnPoint;
 
-                var checkPending = function( newTagName )
+                function checkPending( newTagName )
                 {
                         if ( pendingInline.length > 0 )
                         {
@@ -76 +79 @@
 
                                                 // Add it to the current node and make it the current,
                                                 // so the new element will be added inside of it.
-                                                currentNode.add( pendingElement );
+                                                pendingElement.parent = currentNode;
                                                 currentNode = pendingElement;
 
                                                 // Remove the pending element (back the index by one
@@ -86 +89 @@
                                         }
                                 }
                         }
-                };
+                }
 
+                function addElement( element, target, enforceCurrent )
+                {
+                        target = target || currentNode || fragment;
+
+                        // If the target is the fragment and this element can't go inside
+                        // body (if fixForBody).
+                        if ( fixForBody && !target.type && !CKEDITOR.dtd.$body[ element.name ] )
+                        {
+                                var savedCurrent = currentNode;
+
+                                // Create a <p> in the fragment.
+                                currentNode = target;
+                                parser.onTagOpen( 'p', {} );
+
+                                // The new target now is the <p>.
+                                target = currentNode;
+
+                                if ( enforceCurrent )
+                                        currentNode = savedCurrent;
+                        }
+
+                        target.add( element );
+
+                        if ( element.returnPoint )
+                        {
+                                currentNode = element.returnPoint;
+                                delete element.returnPoint;
+                        }
+                }
+
                 parser.onTagOpen = function( tagName, attributes, selfClosing )
                 {
-                        if ( fixForBody && !currentNode.type && !CKEDITOR.dtd.$body[ tagName ] )
-                                this.onTagOpen( 'p', {} );
-
                         var element = new CKEDITOR.htmlParser.element( tagName, attributes );
 
                         // "isEmpty" will be always "false" for unknown elements, so we
@@ -123 +153 @@
                                 // If the element name is the same as the current element name,
                                 // then just close the current one and append the new one to the
                                 // parent. This situation usually happens with <p>, <li>, <dt> and
-                                // <dd>, specially in IE.
-                                if ( tagName != currentName )
+                                // <dd>, specially in IE. Do not enter in this if block in this case.
+                                if ( tagName == currentName )
                                 {
-                                        // If it is optional to close the current element, then
-                                        // close it at this point and simply add the new
-                                        // element after it.
-                                        if ( !optionalClose[ currentName ] )
+                                        addElement( currentNode, currentNode.parent );
+                                }
+                                else
+                                {
+                                        if ( tableBlocks[ currentName ] )
                                         {
-                                                // The current element is an inline element, which
-                                                // cannot hold the new one. Put it in the pending list,
-                                                // and try adding the new one after it.
-                                                pendingInline.unshift( currentNode );
+                                                if ( !returnPoint )
+                                                        returnPoint = currentNode;
                                         }
+                                        else
+                                        {
+                                                addElement( currentNode, currentNode.parent, true );
 
+                                                if ( !optionalClose[ currentName ] )
+                                                {
+                                                        // The current element is an inline element, which
+                                                        // cannot hold the new one. Put it in the pending list,
+                                                        // and try adding the new one after it.
+                                                        pendingInline.unshift( currentNode );
+                                                }
+                                        }
+
                                         reApply = true;
                                 }
 
                                 // In any of the above cases, we'll be adding, or trying to
                                 // add it to the parent.
-                                currentNode = currentNode.parent;
+                                currentNode = currentNode.returnPoint || currentNode.parent;
 
                                 if ( reApply )
                                 {
@@ -153 +194 @@
 
                         checkPending( tagName );
 
-                        currentNode.add( element );
+                        element.parent = currentNode;
+                        element.returnPoint = returnPoint;
+                        returnPoint = 0;
 
-                        if ( !element.isEmpty )
+                        if ( element.isEmpty )
+                                addElement( element );
+                        else
                                 currentNode = element;
                 };
 
                 parser.onTagClose = function( tagName )
                 {
-                        var closingElement = currentNode,
-                                index = 0;
+                        var index = 0;
 
-                        while ( closingElement && closingElement.name != tagName )
+                        while ( currentNode.type && currentNode.name != tagName )
                         {
                                 // If this is an inline element, add it to the pending list, so
                                 // it will continue after the closing tag.
-                                if ( !closingElement._.isBlockLike )
+                                if ( !currentNode._.isBlockLike )
                                 {
-                                        pendingInline.unshift( closingElement );
+                                        pendingInline.unshift( currentNode );
 
                                         // Increase the index, so it will not get checked again in
                                         // the pending list check that follows.
                                         index++;
                                 }
 
-                                closingElement = closingElement.parent;
+                                addElement( currentNode, currentNode.parent );
+                                currentNode = currentNode.parent;
                         }
 
-                        if ( closingElement )
-                                currentNode = closingElement.parent;
+                        if ( currentNode.type )
+                        {
+                                var node = currentNode;
+                                addElement( currentNode, currentNode.parent );
+
+                                // The parent should start receiving new nodes now, except if
+                                // addElement changed the currentNode.
+                                if ( node == currentNode )
+                                        currentNode = currentNode.parent;
+                        }
                         else if ( pendingInline.length > index )
                         {
                                 // If we didn't find any parent to be closed, let's check the
@@ -223 +276 @@
                         currentNode.add( new CKEDITOR.htmlParser.comment( comment ) );
                 };
 
+                // Parse it.
                 parser.parse( fragmentHtml );
 
+                // Close all pending nodes.
+                while ( currentNode.type )
+                {
+                        var parent = currentNode.parent,
+                                node = currentNode;
+
+                        if ( fixForBody && !parent.type && !CKEDITOR.dtd.$body[ node.name ] )
+                        {
+                                currentNode = parent;
+                                parser.onTagOpen( 'p', {} );
+                                parent = currentNode;
+                        }
+
+                        parent.add( node );
+                        currentNode = parent;
+                }
+
                 return fragment;
         };
 

_source/tests/core/htmlparser/fragment.html

@@ -10 +10 @@
         <script type="text/javascript">
         //<![CDATA[
 
-CKEDITOR.test.addTestCase( (function()
+var tc;
+
+CKEDITOR.test.addTestCase( tc = (function()
 {
         // Local reference to the "assert" object.
         var assert = CKEDITOR.test.assert;
 
+        function testParser( input, expected )
+        {
+                var fragment = CKEDITOR.htmlParser.fragment.fromHtml( input, true ),
+                        writer = new CKEDITOR.htmlParser.basicWriter();
+
+                fragment.writeHtml( writer );
+
+                assert.areSame( expected, writer.getHtml( true ) );
+        }
+
         return {
                 test_fromHtml_1 : function()
                 {
@@ -25 +37 @@
                         assert.areSame( 'p', fragment.children[0].name, 'Wrong child name' );
                 },
 
+                test_parser_1 : function()
+                {
+                        testParser(     '<table><tr><td>1</td><p><b>2</b> Test</p><td>3</td></tr></table>',
+                                                '<p><b>2</b> Test</p><table><tr><td>1</td><td>3</td></tr></table>' );
+                },
+
+                test_parser_2 : function()
+                {
+                        testParser(     '<b><table><tr><td>1</td><td>2</td></tr></table></b>',
+                                                '<table><tr><td><b>1</b></td><td><b>2</b></td></tr></table>' );
+                },
+
+                test_parser_3_1 : function()
+                {
+                        testParser(     '<b><i>Table:<table><tr><td>1</td><td>2</td></tr></table></i></b>',
+                                                '<p><b><i>Table:</i></b></p><table><tr><td><b><i>1</i></b></td><td><b><i>2</i></b></td></tr></table>' );
+                },
+
+                test_parser_3_2 : function()
+                {
+                        testParser(     '<b><i><table><tr><td>1</td><td>2</td></tr></table>Table</i></b>',
+                                                '<table><tr><td><b><i>1</i></b></td><td><b><i>2</i></b></td></tr></table><p><b><i>Table</i></b></p>' );
+                },
+
+                test_parser_4 : function()
+                {
+                        testParser(     '<b><i>Test',
+                                                '<p><b><i>Test</i></b></p>' );
+                },
+
+                test_parser_5 : function()
+                {
+                        testParser(     '<p>Para 1<p>Para 2<p>Para 3',
+                                                '<p>Para 1</p><p>Para 2</p><p>Para 3</p>' );
+                },
+
+                test_parser_6 : function()
+                {
+                        testParser(     '<b>A</b><i>B</i>',
+                                                '<p><b>A</b><i>B</i></p>' );
+                },
+
+                test_parser_7 : function()
+                {
+                        testParser(     '<p>Para 1<hr>Para 2<h1>Para 3',
+                                                '<p>Para 1</p><hr /><p>Para 2</p><h1>Para 3</h1>' );
+                },
+
+                test_ticket_3195 : function()
+                {
+                        testParser(     '<table><tr><td>1</td><p>2</p><td>3</td></tr></table>',
+                                                '<p>2</p><table><tr><td>1</td><td>3</td></tr></table>' );
+                },
+
                 name : document.title
         };
 })() );
 
+// Uncomment the following to run a single test.
+// window.onload = tc.test_parser_7;
+
         //]]>
         </script>
 </head>