Edit definition lists crashes IE 7,8,9

[oyvihatl]

Steps to reproduce:

Go to: ​http://nightly.ckeditor.com/6325/_samples/output_xhtml.html Or: ​http://ckeditor.com/demo

2. Press "Source"

3. Copy into editor:

   <dl>
     <dt>Coffee</dt>
     <dd>- black hot drink</dd>
     <dt>Milk</dt>
     <dd>- white cold drink</dd>
   </dl>
   

4. Press "Source" to view

5. Press "Source" again to edit

Notice:

It works with only <dt>'s inside <dl>:

<dl>
  <dt>Coffee</dt>
  <dt>Milk</dt>
</dl>

Browser name and OS:

IE 7.0.5730.11 - MS Windows Server 2003

[oyvihatl]

The title should say just "Edit definition lists crashes IE 7", as saving only occurs in our CMS.

[Krzysztof Studnik]

Error says "Stack overflow at line 27"
Behaviour confirmed for CKEditor 3.5 and IE7.
In previous versions of Editor, order of items is lost when user is switching back to Source mode, next, the <dt>Coffee</dt> part is gone when Source button is pressed third time.

[Alex (Shurf) Frenkel]

Confirmed for 3.5 and IE8.

6997 is a dublicate please close

[Wiktor Walc]

IE7 enters into an infinite loop here:

parser.onTagOpen.apply( this, arguments );

It looks like we have invalid code in fragment.js:

else if ( tagName in CKEDITOR.dtd.$listItem )
{
	parser.onTagOpen( 'ul', {} );
	addPoint = currentNode;
	reApply = true;
}

Note that $listItem is one of the following {dd:1,dt:1,li:1} and we're using ul for all of them.

[Jakub Ś]

The same thing as on IE7 happens on IE6. I could reproduce it from version 3.4.2

[Sa'ar Zac Elias]

Also occurs in IE8:

fragment.js, line 127 character 4

if ( element.previous !== undefined )

[Garry Yao]

As definition list is not *officially* supported before in the editor, this ticket would be a good start for it ;)

[Frederico Caldeira Knabben]

Right now, let's simply not break because of it, maintaining the HTML structure.

[Frederico Caldeira Knabben]

The fix is ok, but CKEDITOR.dtd is a kind of a "pure" thing in the API, which should be trustable enough and without hacks. Adding $optionalClose to it, implies this new list to have elements like "p", "li" and others.

As we need this check at the fragment code only, let's simply have this element list directly there.

[Garry Yao]

Fixed with [6978].

[meka9233]

The fix did not workd for me, I send the content of a form to CKEDITOR that has tags like this:

<dl>
 <dt><dd>text<dd><dt>
<dt><dd>text<dd><dt>
<dt><dd>text<dd><dt>
</dl>

and as you see the <dt> tags are not closed as they should. The ckeditor cant show the content of the form mailed to this editor and make a stack overflow at line: 26. Is there any way to fix it?

Regards

[Jennifer]

I was directed to this page from bug #6151 ​http://dev.ckeditor.com/ticket/6151

I'm having the exact problem described in ticket #6151, (that dt dd flips position in IE7 and IE8 in compt. mode) but that problem is DIFFERENT than the problem being described here.

Unfortunatly bug #6151 is now closed and the resolution is set to "worksforme" (Works For Me) and the comment saya: Probably fixed at #6975.

Probably fixed here? Resolution set to worksforme?

I'm going to apply this fix and see if it "worksforme" too, but I just wanted to point out this odd method of bug closure on the other page. I wanted to add this comment now in case commenting gets turned off before I can check.

[Jennifer]

My current version of ckeditor, 3.3.2, is too far behind for me to apply this patch directly. I did some comparing of my fragment.js file to the new fragment.js in version 3.6.1 and although they are different and there are some new parameters being passed to some of the functions, I *think* I can replace the entire file without causing too much harm. (crazy I know, but so much has changed I think trying to hack in a solution would be worse)

This one will have to stay in the testing phase for a WHILE, but may be a workable solution until we are able to do a full upgrade.