CKEditor creating hidden spans which can result in hidden content

[Duncan Simey]

One of our customers reported instances of text that looked OK when they typed it in to CKEditor, but was missing when the edited HTML was later included in reports generated by our application. Investigation showed their text was inside spans with style display:none We've worked out how to reproduce the problem with Firefox and IE (only checked IE7 onwards), see below.

The text inside the display:none spans is only displayed inside CKEditor on some browsers (e.g. IE9 in IE7 document mode). If the person entering the text has one of these browsers then there is no indication that there is a problem. The problem can be reproduced on 3.6.2 Demo and the Nightly Build demo.

To reproduce, do the following.... If using IE, turn on JavaScript Alerts using Advanced Internet Options. Paste the following into the editor while in Source mode.

<!-- Start of Text -->
<p>Testing</p>
<p>One</p>
<p>Two</p>
<p>Three</p>
<!-- End of Text -->

Go back to WYSIWYG mode. Select the word Testing and use the toolbar to set a background colour; in IE you get a JavaScript error (note the location of the problem). Select the word Two and use the toolbar to set a background colour; it works fine. Select the word Three and use the toolbar to set a background colour; in IE you get a JavaScript error (the error details indicate a different location to the first error because it is a different problem). Note: The problem with Three does not occur with Firefox.

Inspect the HTML in Source mode; it now contains weird hidden spans (see below). Clicking after the end of the word Testing and typing some text adds the text into the hidden span. Depending on the exact browser version, the typed text either gets displayed or hidden or sometimes does not go into the hidden span.

<!-- Start of Text -->
<p><span id="cke_bm_74S" style="display: none">&nbsp;</span>Testing<span id="cke_bm_74E" style="display: none">&nbsp;</span></p>
<p>One</p>
<p><span style="background-color: #ffff00">Two</span></p>
<p><span id="cke_bm_77S" style="display: none">&nbsp;</span>Three<span id="cke_bm_77E" style="display: none">&nbsp;</span></p>
<!-- End of Text -->

The example above used Background Colour but can be reproduced with any toolbar button which works on selected text (Bold, Font, etc). The HTML comments in the source are what is triggering the problem. During testing we have also observed other ways of producing JavaScript errors and have not yet worked out reproducible test cases. These include an error during paste and an error while changing the selection. Therefore there are at least two more causes of JavaScript errors.

The problem is being caused by CKEditor badly handling JavaScript errors. During command execution the source HTML is marked up with hidden spans indicating the start and end of the selected area. These are being added at the start of the command, but are not being tidied up because the script is being aborted by the error. If this was a simple bit of code, then I'd suggest adding a try/catch block where the exception handler cleans up the hidden spans. Whatever solution is proposed, it needs to be generic enough to cope with a range of problems including ones we have not yet worked out how to repeatably demonstrate!

There is a related ticket #8232 (Inline styles create empty garbage "cke_bm_" spans). This fixed one specific cause of JavaScript error. It did not fix the problem described on that ticket by fastballweb in comment 2. Nor did it fix the general problem of tidying up after exceptions.

[Jakub Ś]

First Problem: JavaScript errors caused by comments.

Reproducible in IE6-9 and Firefox3.6-10 from CKEditor 3.5.1
Message: sibling.is is not a function
URI: /3.6.2/ckeditor/_source/core/dom/range.js
Line: 1032

Reproducible in IE6-9 only from CKEditor 3.5.1 rev [6391] Message: Object doesn't support property or method "is"
URI: /3.6.2/ckeditor/_source/core/dom/range.js
Line: 1192

---

Second problem: As a result of errors invisble spans are inserted

[Jakub Ś]

Similar problem: #8569

Other related to problems with HTML comments: #8640, #8216, #8129, #6709, #8812

[Jakub Ś]

#8687 and #8758 were marked as dulicates.

[Jakub Ś]

Another TC from #8687
Paste <p><!-- BUG -->loren ipsum</p>, go back to WYSIWYG, press ctrl+A, ctrl+B (for bold)

A quick fix propoesed in #8687: URI: core/dom/range.js,
method: enlarge()

if ( ( sibling.$.offsetWidth > 0 || excludeBrs && sibling.is && sibling.is( 'br' ) ) && !sibling.data( 'cke-bookmark' ) )

[Frederico Caldeira Knabben]

I'm not able to reproduce this issue any more with the current trunk.

[Jakub Ś]

This issue is still reproducible in IE

1. Paste the below

   <!-- Start of Text -->
   <p>Testing</p>
   <p>One</p>
   <p>Two</p>
   <p>Three</p>
   <!-- End of Text -->
   

2. Change background color of word Testing and then try to change background color of word Three

You will get:
Reproducible in IE6-9 only from CKEditor 3.5.1 rev [6391] Message: Object doesn't support property or method "is"
URI: /trunk/ckeditor/_source/core/dom/range.js
Line: 1198

[Frederico Caldeira Knabben]

Replying to j.swiderski:

This issue is still reproducible in IE

Confirmed, but note that the tc you have reported is different than the original ticket tc and than its simplified tc.

[Duncan Simey]

Fred - There are lots of ways to reproduce javascript problems. I noted we head spotted others I'd failed to reproduce reliably. Which is why I asked for a general try / catch type solution instead of quick fixes. I still think better exception handling is the way forward.

[Frederico Caldeira Knabben]

Replying to duncansimey:

Fred - There are lots of ways to reproduce javascript problems. I noted we had spotted others I'd failed to reproduce reliably. Which is why I asked for a general try / catch type solution instead of quick fixes. I still think better exception handling is the way forward.

I got your point, but the problem here is that, if really doable, this would involve a strong refactoring on good part of our code. We would have to create a "job-based" system that would revert the job in case of issues, reporting the error to the user (and eventually to us). That's all cool, but is something to consider in a dedicated ticket (#8825).

In this ticket instead, we have a specific issue identified. The system we just described would still not help here. It would just avoid the js error, but the user would still not be able to do what s/he was intended to do. So, a specific patch is needed.

[Duncan Simey]

Thanks Fred. We will work out steps to create some of the other problems we have seen, so they can be fixed too.

[Garry Yao]

​http://ckeditor.t/tt/8732/1.html

[Frederico Caldeira Knabben]

Fixed with [7406].