Error when indenting and outdenting empty list elements

[Jon Håvard Gundersen]

Scenario:
1. Go to sample01 nightly build and clear content
2. Make a list with two elements
3. Go to the end of the first element and press enter to make a new empty list element between the other two.
4. Press indent button.
5. Press outdent button.
6. The list is now completely broken.

Tested with IE7
It seems to have something to do with the end of line charachter beeing selected after indentation. This makes a lot of unexpected results in other cases too.

[Jon Håvard Gundersen]

Other related strange behaviours:

At step 5:

1. Press numbered list instead of bullet list causes the last element to change from bullet list to numbered list.

2. Press enter - a new element are made on root level and the cursor is moved to the last element.

3. Write some text - the list element are deleted and the text are added to the first element.

[Martin Kou]

Fixed with [1064].

Click here for more info about our SVN system.

[Jon Håvard Gundersen]

This bug is not completely crushed.

Instead of pressing outdent button in step 5. Try to write some text. You will see that this text is added to the list element above the current element. The current element seems to be deleted.

As I see it the problem is that the indented list element is selected after indentation.

[Jon Håvard Gundersen]

Another related scenario:
1. Empty content in sample01.html
2. Add two list elements with text
3. Add new list element and indent it, write text without clearing selection.
4. Add new element to the sublist. Try to indent it - result: javascript error: 'indent' is null or not an object.

If I deselect the selection before writing text in step 3 this error does not accur.

[Jon Håvard Gundersen]

After a lot of testing it seems like the problem is located at line 74 in fckindentcommands.js where FCK.Focus() is called after the list element creation. When I comment out this line the new list element is no longer selected and everything seems to be working..

I don't know the reason this method is called, but maybe you could just drop this call?

[Jon Håvard Gundersen]

To be more specific - it seems to be inside FCKEditingArea._EnsureFocusIE();

[Jon Håvard Gundersen]

The problem is the kludge code added for #141 with [621] / [623]. This code should ensure correct focus in IE and that the cursor should be inside the first element and not between body and the first child element.

My testing show that this kludge does not work anymore, the cursor is now always placed before the first child element on load (very annoying) and just causes a lot of trouble with lists.

[Martin Kou]

I've fixed the list item selection problem in [1082].

I couldn't get the cursor to appear like |<p>&nbsp;<p> as you said in #141 though. How did you trigger the bugged behavior? I've tried setting StartupFocus = true and tested with sample01.html with a few different default values (e.g. oFCKeditor.Value = '<p>&nbsp;</p>'). I haven't seen the caret appearing before the first block element.

[Jon Håvard Gundersen]

First of all: I'm using IE6.

When I try setting startupFocus = true in sample01.html I still get the same problem as described before. The cursor is placed outside the paragraph. This causes several unwanted behaviours. I.e. When I start to write text, the existing paragraph jumps down a line.

When I debug the content of the editor with IE developer toolbar I can see that the text is placed directly into the body element. This is of course fixed after a enter press or when switching editing mode.

[Martin Kou]

I can't reproduce the problem on my local machine anyway but I've got some friends who told me he could occasionally see it happen. Tracing the logic a bit shows that the kludge in _EnsureFocusIE() isn't executed when FCKeditor was loaded with StartupFocus = true, and I guess that's the reason you're getting the bug.

The kludge in _EnsureFocusIE() isn't running because of the changes in [799]... I've changed the kludge's logic in [1084] such that:

1. It won't select the "end of line" character in IE anymore, and
2. It will be executed at startup when StartupFocus=true.

This should fix the bug you've encountered while not breaking previously fixed bugs (e.g. #1178). Since I can't reproduce your bug in the first place, could you tell me if this fix works on your side?

[Jon Håvard Gundersen]

I've copied the changes you made in fckeditingarea.js to my local version and everything seems to be working fine now! The list isn't selected and the caret is moved inside the first paragraph on startup.

Good work!

btw, I guess you can remove the oldlength variable at line 313 which is not in use anymore..

[Martin Kou]

Oh yes, good observation, I missed that one. Fixed that with [1087]. :)

Closing ticket.

[Frederico Caldeira Knabben]

It seems like [1064] is not anymore needed for this ticket. Am I right? Should we revert it? Can you provide a TC that shows that it's needed?

[Frederico Caldeira Knabben]

Also, it it is supposed to be used, there is a small improvement to it:

	if ( /^\s*<LI><\/LI>$/.test( htmlText ) )

[Martin Kou]

You're right, [1064] is no longer needed. It was intended only for fixing the issue originally reported by jonhg, and [1084] is a better solution for it.

I've reverted the unnecessary changes in [1118].

[Frederico Caldeira Knabben]

To fix #1587, I had to partially revert [1084]. I've also changed part of its logic, to be sure it will not interfere in this ticket.

Can martin and johng confirm it is still working properly after my change?

[Martin Kou]

I've tested the updated code again with johng's test cases and I didn't find any problem so far.

[Jon Håvard Gundersen]

This revert causes the bug with startupfocus to reappear in my testcase. When writing text directly after loading the editor the text is placed under the body and not inside a paragraph.

[Jon Håvard Gundersen]

As I see it the problem is the test 'parentNode.childNodes.length > 0' this will always return true if the parentNode contains text, both #1587 and this ticket seems to be working ok if I alter the test to 'parentNode.childNodes.length > 1'.

You can maybe make a more elegant fix...

[Martin Kou]

Replying to jonhg:

As I see it the problem is the test 'parentNode.childNodes.length > 0' this will always return true if the parentNode contains text, both #1587 and this ticket seems to be working ok if I alter the test to 'parentNode.childNodes.length > 1'.

You can maybe make a more elegant fix...

I've added an extra condition check in [1136] to hopefully fix the problem. Can you confirm whether this fix works or not on your side?

[Frederico Caldeira Knabben]

Replying to jonhg:

This revert causes the bug with startupfocus to reappear in my testcase. When writing text directly after loading the editor the text is placed under the body and not inside a paragraph.

It would be nice to have a TC for it, because we are not able to consistently reproduce it here.

[Jon Håvard Gundersen]

Replying to martinkou:

Replying to jonhg:

As I see it the problem is the test 'parentNode.childNodes.length > 0' this will always return true if the parentNode contains text, both #1587 and this ticket seems to be working ok if I alter the test to 'parentNode.childNodes.length > 1'.

You can maybe make a more elegant fix...

I've added an extra condition check in [1136] to hopefully fix the problem. Can you confirm whether this fix works or not on your side?

When debugging I have found thath the parentTag is P so it would not help to check if the parent tag is body, but that was my first thought too. It seems to be some weird IE problem, the range is inside the P, but moved outside when writing. Checking for childNodes.length > 1 does not break bold and places the cursor inside the paragraph as expected.

I can try to make a TC tomorrow, in what way do you want it?

[Frederico Caldeira Knabben]

I've reverted [1136] with [1137].

johng, I'm closing this ticket because the issue you are having has nothing to do with it. Not even #141 seems to be related as we are simply talking about the focus there.

So, I would ask you to open a dedicated ticket for it, providing a TC so we can reproduce it here. A TC would be a simple HTML file with an editor instance on it (like sample01.html) which consistently shows the problem. This is needed because there could be even external factors interfering in the editor... a classic IE joke with us.

It is important to have in mind that _EnsureFocusIE is there to "ensure the focus", not to place the caret inside the paragraph. Of course it is ok to use it to solve both problems, but we must remember that it is also there for other purposes.

Note: tomorrow is the release date for the 2.5.