CKEditor exception in Firefox

[Alan]

Please see comment:4 and comment:5 for more details.

Our web app has been generating countless exceptions in CKEditor. I have finally been able to reproduce one of them. For the demo please visit '​http://jsfiddle.net/ftey46fc/3/' and follow these steps in the editor in the lower right:

1) Type three lines of text with the letters a, b, and c on them (one

letter per line).

2) Select the first two lines and make that a numbered list. 3) Select all three lines and change to a bulleted list.

An exception is generated "TypeError: a is null".

This error is caused because when bookmarks are selected/restored in the selectBookmarks function the starting bookmark has been removed thus it fails to select it in the moveToBookmark function so startNode is null and the setStartBefore call will a null parameters throws an exception. I changed said function during me debugging to:

setStartBefore: function (node) {

if (node == null) {

console.log('* About to throw an exception');

} this.setStart( node.getParent(), node.getIndex() );

}

to better illustrate that.

Why has the bookmark been purged? Well the long story is that a <li> with the start bookmark gets added before the two existing <li>s and so the first <li> has no text in it and it's purged by the 'changeListType' function, but it all seems to come down to the getNative function that looks kind of (this is my debug version now) like this:

getNative: function() {

if ( this._.cache.nativeSel !== undefined ){

return this._.cache.nativeSel;

} if(isMSSelection){

this._.cache.nativeSel = this.document.$.selection;

}else{

var win = this.document.getWindow(); this._.cache.nativeSel = win.$.getSelection();

}

console.log('* new native selection = anchor = ' + this._.cache.nativeSel.anchorNode.nodeName + ' focus node = ' + this._.cache.nativeSel.focusNode.nodeName); return ( this._.cache.nativeSel );

}

which I have modified to break complex lines into pieces and add the logging. When this returns the selection via getSelection the focus node SHOULD be on the 'a' #text node, but it's actually on the OL node. This works fine in Chrome and usually works fine in FireFox, but the CSS I inject into the document seems to trigger this problem. Remove that and it works fine.

That makes me question whether this is actually a Firefox bug that needs to be worked around, but I don't know my browser specs. In any event one quick fix I've done to code around this was a patch to the moveToBookmark function that basically verifies that the startBookmark was found but

a) I don't know the extent that this kind of bug is going to have

on the rest of the system. If there are other places where the start bookmarks premature removal is going to wreak havoc then I'd like to know about it.

b) I don't know if there are other cases where we're going to see this

type of problem because of other CSS that Firefox/CKEditor doesn't work with properly. Like I said we're seeing all kinds of exceptions coming from CKEditor and it could all be permutations of this bug. I don't know yet.

c) When the exception occurs it leaves the end bookmark span in the

HTML and it seems that that can cause later problems.

Let me know if there's any additional info I can provide. I'm just glad to finally get this bug in a form reproducible outside of our code base.

• Thanks, Alan

[Alan]

Now I see at least one reason this hasn't had a response... I posted the wrong demo link. It should be:

​http://jsfiddle.net/ftey46fc/12/

[Jakub Ś]

Hi. I have checked your fiddle but didn't get this error. I have tried in Firefox, Blink and IE.

My TC steps (Have I done something wrong here?):

1. Type a, Enter, b, Enter, c
2. Select a,b with mouse and make is numbered list
3. Click next to a element to remove selection (I have also tried not removing existing selection and select last element with keyboard)
4. Make all a bullet list

Result: Bullet list and no errors.

I have tried in Windows and Mac.

---

Any tips telling me how to reproduce this problem are more than welcome.

[Alan]

If you're not seeing the exception then you're not doing it correctly :-)

1. No need to try other browsers... FireFox is all you need.
2. I'm running Windows 7 to do this.
3. Not sure about your step 3... you will see an exception in the console log after selecting the first two lines and creating a numbered list and then selecting all three lines (a and b being part of a numbered list and c being a separate <p>) and then converting that to a bulleted list.

I am in the process of uploading a video of what I am doing to YouTube so hopefully that will clarify the steps needed to recreate this exception.

[Alan]

At ​http://youtu.be/FEQBQOQsknU you will see a video of the bug starting with navigating to ​http://jsfiddle.net/ftey46fc/12/ in FireFox.

0:00 I show the URL of the page (​http://jsfiddle.net/ftey46fc/12/)

0:10 Show Firefox version is current (36.0.1)

0:24 Show current status of the web console

0:35 Type lines a, b, and c into the editor

0:41 Select the three lines (not necessary - this was an accident)

0:45 Select the first two lines

0:48 Click on numbered list button

0:50 Select all three lines

0:54 Show web console and that no exceptions have been generated

1:00 Click on bulleted list button

1:06 You can see the triggered exception in the console log

• Thanks, Alan

[Jakub Ś]

I was able to reproduce this issue from CKEditor 4.0 beta in Firefox.

Error:
Message: node is null
Code: this.setStart( node.getParent(), node.getIndex() );
URI: range.js
Line: 1765

The important things are inserting styles into CKEditor document header and selecting list from bottom to top. Another important thing to note is that removing 'list-style: inside;' solves the problem

[Alan]

Yes, the CSS styles are critical for reproducing this.

And I think the root of the problem is when the browser gets the native selection the browser is reporting wrong stuff. If you trace through it you'll see that when the bookmark is created it inserts a new <li><span [bookmark span] /></li> into the list (that is later deleted because it has no text in it) so the bookmark is being created in the wrong place. And I think that happens because the browser's native selection is wrong, but I'll let you be the judge of that.

Any suggestions on how we can fix this temporarily? For now we're considering adding this patch so that it ignores the missing start bookmark... however we have a lot more exceptions being triggered (the latest happens when the user presses backspace or delete, but I have to figure out the circumstances for that) and if the browser is reporting bad stuff then I could see it as likely that that's the root cause of many, if not all, of our issues. It would be nice to get a workaround for that :-)

• Thanks, Alan

    // This patch was created by taking the version of the function from CKEditor 4.4.3 and making a minor change
    //  as noted at the bottom in an attempt to resolve some exceptions
    CKEDITOR.dom.range.prototype.moveToBookmark = function (bookmark) {
        if (bookmark.is2) // Created with createBookmark2().
        {
            // Get the start information.
            var startContainer = this.document.getByAddress(bookmark.start, bookmark.normalized),
                startOffset = bookmark.startOffset;

            // Get the end information.
            var endContainer = bookmark.end && this.document.getByAddress(bookmark.end, bookmark.normalized),
                endOffset = bookmark.endOffset;

            // Set the start boundary.
            this.setStart(startContainer, startOffset);

            // Set the end boundary. If not available, collapse it.
            if (endContainer)
                this.setEnd(endContainer, endOffset);
            else
                this.collapse(true);
        } else // Created with createBookmark().
        {
            var serializable = bookmark.serializable,
                startNode = serializable ? this.document.getById(bookmark.startNode) : bookmark.startNode,
                endNode = serializable ? this.document.getById(bookmark.endNode) : bookmark.endNode;

            // This is the only change to this function... basically we're validating that we REALLY
            //  found a startNode before trying to use it. It has been found that some CSS causes the
            //  editor to report an incorrect selection in FF which for the following case (and possibly others)
            //  causes the start node to be deleted before it can be processed here:
            //
            // Starting with an empty editor create three lines with the letters a, b, and c on them
            // Select the first two lines (a and b) and make them into a ordered/numbered list.
            // Select all three lines and convert it to an unordered list.
            // 
            // That should trigger an exception. The root of this problem is actually in the CKEDITOR.dom.selection.getNative function
            //  when it tries to get the native selection from the browser when a cached selection is not available. Ticket #13020
            //  was opened in the CKEditor bug reporting system as a correct solution should involve somehow correcting this
            //  native selection.
            //
            // Please also note that during selection and testing of various possible fixes different behavior was observed when
            //  selecting from the last line (c) to the first line (a) when ending one's selection after the '1.', before the '1.',
            //  and outside of the editor's border.
            //
            if (startNode) {
                // Set the range start at the bookmark start node position.
                this.setStartBefore(startNode);

                // Remove it, because it may interfere in the setEndBefore call.
                startNode.remove();
            }

            // Set the range end at the bookmark end node position, or simply
            // collapse it if it is not available.
            if (endNode) {
                this.setEndBefore(endNode);
                endNode.remove();
            } else
                this.collapse(true);
        }
    }

[Alan]

And here's a video of this exception happening on your main demo site:

​http://youtu.be/gEco9p7r4uU

It's not just special CSS that is causing this.