Opened 11 years ago

Closed 11 years ago

Last modified 9 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 11 years ago.
2368_2.patch (1.6 KB) - added by Martin Kou 11 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 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 11 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 11 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

comment:4 Changed 11 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 11 years ago by Martin Kou

Attachment: 2368.patch added

comment:5 Changed 11 years ago by Martin Kou

Keywords: Review? added

comment:6 Changed 11 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 11 years ago by Martin Kou

Attachment: 2368_2.patch added

comment:7 Changed 11 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:8 Changed 11 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:9 Changed 11 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 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy