[IE] Can't remove styles after calling GetData()

[Patrick McElhaney]

In IE7, open the first sample. (​http://www.fckeditor.net/demo)

Call GetXML() by pasting this code into the address bar.

javascript:alert(FCKeditorAPI.GetInstance('FCKeditor1').GetData());

You should get an alert with the source currently in FCKEditor. Dismiss the alert.

Now try making the text that's bold not bold. It doesn't work. (I've tried selecting the bolded text and clicking the "B" button. I've also tried placing the cursor inside the bolded text an clicking the "B" button. These both work fine prior to calling GetData().)

[Wojciech Olchawa]

Confirmed both in IE6 and IE7. Works fine with FF. Tested on FCKeditor stable 2.6 and the latest SVN.

[Patrick McElhaney]

This bug was introduced with the new style system (r774). It works fine in r772.

[Patrick McElhaney]

Switching to source view and back clears up the problem.

[Patrick McElhaney]

I figured out what's happening. GetData() leaves _fckhtmljob attributes behind, which in turn triggers bug #2092.

[Blake Matheny]

Any updates on this bug? It's been open for 9 months, it's still valid in 2.6.3, and no commits have yet been made to fix it. Is there a workaround?

[Patrick McElhaney]

Changed the version to 2.5 Beta, since that's the first release in which it appeared. Still valid as of 2.6.4 Beta.

[P.J. Hinton]

Replying to pmcelhaney:

I figured out what's happening. GetData() leaves _fckhtmljob attributes behind, which in turn triggers bug #2092.

I have reproduced the issue described in the quoted comment. Here is a more detailed discussion of what is happening along with some proposed fixes.

When FCK.GetData() is called, FCKXHtml._AppendNode() gets invoked as part of an operation that copies the editor's internal HTML DOM document to an XML document that will be used to create the XHTML source string. FCKXHtml._AppendNode() adds a property, named _fckxhtmljob to each DOM node object that corresponds to an element. These properties are not removed after the copy is complete.

The presence of these properties has an adverse side effect in Internet Explorer. The function FCK.Style.RemoveFromRange() does not remove a styling element (e.g. strong) when a call to FCKDomTools.HasAttributes() returns true. A true result is obtained because Internet Explorer includes the _fckxhtmljob property in the styling element's attributes NamedNodeMap, and the Attr object corresponding to the _fckxhtmljob attribute has the specified property set to true.

The equivalence between an element object's property and an element's attributes is documented in the MSDN docs for HTML and CSS:

​http://msdn.microsoft.com/en-us/library/ms533026(VS.85).aspx#Accessing_Element_Pr

I have attached two patch files that contain separate candidate fixes for this problem. The diffs are based on the current state of trunk sources as this comment was being written.

The first patch defines a new function in fckxhtml_ie.js called FCKXHtml._RemoveXHtmlJobProperties(). It recursively walks a DOM node object and its descendants to remove the _fckxhtmljob properties. It is invoked by FCKXHtml.GetXHTML() once the document copy operation is complete. It is called only when the browser is Internet Explorer.

The second patch modifies FCKDomTools.HasAttributes() in fckdomtools.js so that it skips over all attributes whose names start with _fck when the browser is Internet Explorer. This code builds upon an existing conditional that applies for IE. The use of StartsWith( '_fck') is based on its use in FCKXHtml._AppendAttributes().

To me, the first patch seems like the Right Thing to do because it forces FCKXHtml to clean up after itself. However, the second approach might be safer in situations where FCK.GetData() being called in the background. Theoretically, one could have a scenario when a user is attempting to perform a style change while an interval callback invokes GetData() to perform an autosave on the content.

If you have any comments or questions on these patches, please feel free to e-mail me. My employer, Compendium Blogware, uses FCKeditor in its application and ​counts itself among those who have been affected by this bug.

Thanks!

[P.J. Hinton]

first candidate fix described in comment 7 of this ticket

[P.J. Hinton]

second candidate fix described in comment 7 of this ticket

[P.J. Hinton]

Adding 'Review?' keyword to this ticket as I failed to set this value after I uploaded the patches. I didn't realize this was necessary until reviewing the ticket instructions on the wiki.

[Frederico Caldeira Knabben]

This is something we are not being able to reproduce in V3, at least for now (some style features are still to be ported there).

I'm targeting it to the next V2 release, as the fix looks good.

[William Echlin]

I'm afraid I can't get either of these patches to work on V2. I have applied patch 2 here....

​http://207.158.15.141/web/test_cases_modify_content.php?id=2 user: admin password: admin

But still when I use IE6, highlight bold text and try to remove it by clicking the bold button nothing happens.

Also tried patch 1 but that had didn't work either.

Any help with this would be very much appreciated.

[Alfonso Martínez de Lizarrondo]

reviewed and merged patch

[Alfonso Martínez de Lizarrondo]

The previous patches did have some bugs: the first one wasn't really removing the fake attribute (it used another name and setting it to null doesn't clear it in IE) and the second one broke the else part of the loop, now IE never executed that.

I've merged both patches (so the code has all the protections) and now it's working correctly for me.

Ups, I forgot the thanks in the what's new to Compendium Blogware, I'll add it in the checkin when the patch is reviewed.

[Alfonso Martínez de Lizarrondo]

#3479 has been marked as dup

[Alfonso Martínez de Lizarrondo]

Fixed with [3882]

[Wiktor Walc]

Unfortunately along with the new function FCKXHtml._RemoveXHtmlJobProperties() a new bug has been introduced, I have created a separate ticket:

#4642

Invalid HTML causes infinite loop in IE