#6975 closed Bug (fixed)
Edit definition lists crashes IE 7,8,9
Reported by: | oyvihatl | Owned by: | Garry Yao |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 3.6.1 |
Component: | Core : DTD | Version: | 3.4 |
Keywords: | IE | Cc: | mehrdad.kavousi@… |
Description
Steps to reproduce:
Go to: http://nightly.ckeditor.com/6325/_samples/output_xhtml.html Or: http://ckeditor.com/demo
- Press "Source"
- Copy into editor:
<dl> <dt>Coffee</dt> <dd>- black hot drink</dd> <dt>Milk</dt> <dd>- white cold drink</dd> </dl>
- Press "Source" to view
- 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
Attachments (2)
Change History (19)
comment:1 Changed 14 years ago by
comment:2 Changed 14 years ago by
Component: | General → UI : Source View |
---|---|
Keywords: | IE7 added |
Status: | new → confirmed |
Summary: | Edit and saving definition lists crashes IE 7 → Edit definition lists crashes IE 7 |
Version: | → 3.4 |
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.
comment:4 Changed 14 years ago by
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.
comment:5 Changed 13 years ago by
The same thing as on IE7 happens on IE6. I could reproduce it from version 3.4.2
comment:6 Changed 13 years ago by
Milestone: | → CKEditor 3.6.1 |
---|
comment:7 Changed 13 years ago by
Keywords: | IE added; IE7 removed |
---|
Also occurs in IE8:
fragment.js, line 127 character 4
if ( element.previous !== undefined )
comment:8 Changed 13 years ago by
As definition list is not *officially* supported before in the editor, this ticket would be a good start for it ;)
comment:9 Changed 13 years ago by
Right now, let's simply not break because of it, maintaining the HTML structure.
Changed 13 years ago by
Attachment: | 6975.patch added |
---|
comment:10 Changed 13 years ago by
Component: | UI : Source View → Core : DTD |
---|---|
Owner: | set to Garry Yao |
Status: | confirmed → review |
comment:11 Changed 13 years ago by
Status: | review → review_failed |
---|
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.
Changed 13 years ago by
Attachment: | 6975_2.patch added |
---|
comment:12 Changed 13 years ago by
Status: | review_failed → review |
---|
comment:13 Changed 13 years ago by
Status: | review → review_passed |
---|
comment:14 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed with [6978].
comment:15 Changed 13 years ago by
Cc: | mehrdad.kavousi@… added |
---|---|
Summary: | Edit definition lists crashes IE 7 → Edit definition lists crashes IE 7,8,9 |
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
comment:16 Changed 13 years ago by
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.
comment:17 Changed 13 years ago by
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.
The title should say just "Edit definition lists crashes IE 7", as saving only occurs in our CMS.