Opened 17 years ago

Closed 17 years ago

#1491 closed Bug (fixed)

Error when indenting and outdenting empty list elements

Reported by: Jon Håvard Gundersen Owned by: Martin Kou
Priority: Normal Milestone: FCKeditor 2.5
Component: General Version: FCKeditor 2.5 Beta
Keywords: Confirmed IE Cc:

Description

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.

Change History (25)

comment:1 Changed 17 years ago by Martin Kou

Keywords: Confirmed IE added
Milestone: FCKeditor 2.5
Owner: set to Martin Kou
Status: newassigned
Version: FCKeditor 2.5 Beta

comment:2 Changed 17 years ago by 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.
  1. Press enter - a new element are made on root level and the cursor is moved to the last element.
  1. Write some text - the list element are deleted and the text are added to the first element.

comment:3 Changed 17 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [1064].

Click here for more info about our SVN system.

comment:4 Changed 17 years ago by Jon Håvard Gundersen

Resolution: fixed
Status: closedreopened

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.

comment:5 Changed 17 years ago by 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.

comment:6 Changed 17 years ago by 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?

comment:7 Changed 17 years ago by Jon Håvard Gundersen

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

comment:8 Changed 17 years ago by 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.

comment:9 Changed 17 years ago by 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.

comment:10 Changed 17 years ago by 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.

comment:11 Changed 17 years ago by 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?

comment:12 Changed 17 years ago by 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..

comment:13 Changed 17 years ago by Martin Kou

Resolution: fixed
Status: reopenedclosed

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

Closing ticket.

comment:14 Changed 17 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: closedreopened

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?

comment:15 Changed 17 years ago by Frederico Caldeira Knabben

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

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

comment:16 Changed 17 years ago by Martin Kou

Resolution: fixed
Status: reopenedclosed

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].

comment:17 Changed 17 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: closedreopened

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?

comment:18 Changed 17 years ago by Martin Kou

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

comment:19 Changed 17 years ago by Martin Kou

Resolution: fixed
Status: reopenedclosed

comment:20 Changed 17 years ago by Jon Håvard Gundersen

Resolution: fixed
Status: closedreopened

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.

comment:21 Changed 17 years ago by 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...

comment:22 in reply to:  21 ; Changed 17 years ago by 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?

comment:23 in reply to:  20 Changed 17 years ago by 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.

comment:24 in reply to:  22 Changed 17 years ago by 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?

comment:25 Changed 17 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: reopenedclosed

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.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy