Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#2368 closed Bug (fixed)

IE: Protected source for comments is broken

Reported by: Frederico Caldeira Knabben Owned by: Martin Kou
Priority: Normal Milestone: FCKeditor 2.6.3
Component: General Version: SVN (FCKeditor) - Retired
Keywords: Confirmed IE Review+ Cc: Scott McNaught

Description

The fix for #2263 is breaking source protection in IE SVN nightly.

To replicate:

  1. Click source.
  2. Add <!-- test --> to the source.
  3. Press Source again to revert to design mode.
  4. Press Source again, and you have a "null" cannot be null.

Attachments (2)

2368.patch (1.5 KB) - added by Martin Kou 9 years ago.
2368_2.patch (1.6 KB) - added by Martin Kou 9 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 9 years ago by Frederico Caldeira Knabben

The following is a proposal fix for the part of the code that is throwing the error:

Index: D:/FCKeditor/SVN/FCKeditor/local/editor/_source/internals/fck.js
===================================================================
--- D:/FCKeditor/SVN/FCKeditor/local/editor/_source/internals/fck.js    (revision 2221)
+++ D:/FCKeditor/SVN/FCKeditor/local/editor/_source/internals/fck.js    (working copy)
 -1111,8 +1111,10 @@
                {
                        if ( typeof( this.Elements[i] ) == 'string' )
                        {
-                               node.innerHTML = this.Elements[i] ;
-                               this.Elements[i] = node.firstChild ;
+                               // For IE, we need to do this trick to avoid loosing comment
+                               // only HTML.
+                               node.innerHTML = '<div>&nbsp;</div>' + this.Elements[i] ;
+                               this.Elements[i] = node.removeChild( node.childNodes[1] ) ;
                        }
                }
        }

But then, there are still other things to get fixed to make it work.

comment:2 Changed 9 years ago by Frederico Caldeira Knabben

Sorry, Trac didn't like my inline diff... here you have it: http://pastebin.com/f637ed2bf

comment:3 Changed 9 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

comment:4 Changed 9 years ago by Martin Kou

Ok... apart from the weird IE bug in which you can't put a bare comment inside a node and expect it to appear in the DOM... there's also the issue that FCKeditor puts protected code as plain text into FCKTempBin, and those plain text should never be converted to DOM nodes at all.

Proposing a patch to fix both issues.

Changed 9 years ago by Martin Kou

Attachment: 2368.patch added

comment:5 Changed 9 years ago by Martin Kou

Keywords: Review? added

comment:6 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The proposed patch is ok. In my previously proposed patch I got also the chance with this fix to use removeChild to retrieve the final element so we release the reference to the temporary node. Couldn't it be used here too? (basically node.firstChild.removeChild( node.firstChild.lastChild ) at line 1118)

Changed 9 years ago by Martin Kou

Attachment: 2368_2.patch added

comment:7 Changed 9 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:8 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:9 Changed 9 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [2258].

Click here for more info about our SVN system.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy