Bugs in JavaScript undo/redo logic of FCKeditor

[Martin Kou]

Background: The JavaScript undo/redo logic is used for IE only in the release version (2.4.3), and is being ported to Firefox in the SVN trunk. FCKeditor's current undo/redo logic in JavaScript suffers from a number of critical bugs.

- Pressing Ctrl-Z breaks the undo/redo system

1. Open the current release version of FCKeditor in http://www.fckeditor.net/demo in Internet Explorer 6 or 7.
2. Typing some random characters into the editor instance.
3. Press Ctrl-Z to undo, it works.
4. Press Ctrl-Y to redo, it doesn't work.

1. Open the current release version of FCKeditor in http://www.fckeditor.net/demo in Internet Explorer 6 or 7.
2. Select the editor instance. Don't type anything yet.
3. Press Ctrl-Z a few times.
4. Type some random characters into the editor instance.
5. Perform undo by either Ctrl-Z or by the toolbar button above, it doesn't work.

There is obviously a logic error in the current undo/redo JavaScript code. If you print out the FCKUndo.CurrentIndex counter after the above test cases, you'll see a negative index lower than -1 (say, -15), which should not happen at all considering it's an index to an array. What really happened was that, each time you pressed Ctrl-Z inside of the editor, the FCKUndo.CurrentIndex was decremented by 1 unconditionally.

- Selection is not restored for the first change

1. Open the current release version of FCKeditor in http://www.fckeditor.net/demo in Internet Explorer 6 or 7.
2. Select some text.
3. Change the font size.
4. Press the undo button in toolbar.
5. The font size is restored, but the text selection is not.

This bug occurs because the current undo/redo logic thinks the snapshot just before the font size change was the same as the initial snapshot taken at editor initialization, so it ignored the newer snapshot. The current logic neglected to check for differences in selection.

- No stepwise undo for Cut and Delete operations

1. Open the current release version of FCKeditor in http://www.fckeditor.net/demo in Internet Explorer 6 or 7.
2. Select some text.
3. Press Ctrl-X or Del.
4. Select some more text.
5. Press Ctrl-X or Del.
6. Select even more text.
7. Press Ctrl-X or Del.
8. Press the undo toolbar button.
9. Your three delete/cut operations were restored in one step. Compare this behavior to MS Word 2003, MS Word restored the 3 operations in a stepwise manner. Also compare the how the stepwise undo you got if you did the cut operation by the toolbar button instead.

This bug occurs because there are currently no registered commands for Ctrl-X or Del keystrokes in FCKeditor. They are treated as simple keyboard input in front of the undo/redo system.

[Martin Kou]

More bugs discovered

- Pressing function keys breaks the undo/redo system

1. Open the current release version of FCKeditor in http://www.fckeditor.net/demo in Internet Explorer 6 or 7.
2. Press the left and right arrow keys for more than 25 times.
3. The undo toolbar button becomes active
4. Press the undo toolbar button.
5. Type some random characters in the document.
6. Press the undo toolbar button.
7. The changes in the document cannot be undone.

The undo toolbar button in step 3 should not have become active. Pressing it causes the FCKUndo.CurrentIndex to go below -1, which breaks the undo/redo system. The design of the current undo/redo system is very fragile.

- Style changes cannot be undone in Firefox See #420 and #2

I've merged the ticket #2 with this ticket.

[Martin Kou]

Fixed with [400].

Click here for more info about our SVN system.

[Alfonso Martínez de Lizarrondo]

Hi martinkou

Nice job fixing the bug and bringing these improvements, just two things:

You added a new file fckundo.js and removed the corresponding browser specific files, but you didn't update the fckpackager.xml file so now the packaged version doesn't work (I did the same mistake), in other cases you might have also to change the fckscriptloader.js file, but it seems that the undo system isn't included there for the moment

Is the new code tested in Opera and/or Safari? (just asking)

I hope that you get this message between all the spam from that bas.

Regards

[Martin Kou]

Ok, I've just updated fckpackager.xml with [402]. The packaged version should work now.

I haven't tested the code on Opera and Safari at the time I closed the ticket. I did some tests today and some of the new code seems to be working on those browsers, but not all. I'll see if it can be made to work on Safari and Opera, then.

[Martin Kou]

I've tested the new code with Opera, and it seems to be working perfectly. I've discovered a bug in Opera during the testing though, and I've filed that under [927].

On Safari though, it seems the browser built-in design mode's default undo/redo logic is used instead of our coded logic.

My test cases for testing the undo/redo system are as follows:

1. Select text, change style, undo
	Expected results: 
	- text style restored, 
	- selection restored.

2a. Type one ANSI charater in the editor after it's loaded
	Expected results: 
	- undo button is enabled, 
	- undo can be performed	by clicking the toolbar button
	- undo can be performed by Ctrl-Z
2b. Undo once
	Expected results: 
	- redo button is enabled,
	- redo can be performed by clicking the toolbar button 
	- redo can be performed by Ctrl-Y
	- previous contents restored
	- previous caret position restored
2c. Redo once
	Expected results:
	- undo button is enabled, 
	- undo can be performed	by clicking the toolbar button
	- undo can be performed by Ctrl-Z
	- changed contents restored
	- changed caret position restored

3. Type more than 25 ANSI characters in the editor
	Expected results:
	- One undo level should be generated for every 25 characters typed

4. Pressing Ctrl-Z repeatedly after the undo button greyed out
	Expected results:
	- Nothing visible happens
	- Undo/redo system should still work normally afterwards

5. Pressing Ctrl-Y repeatedly after the redo button greyed out
	Expected results:
	- Nothing visible happens
	- Undo/redo system should still work normally afterwards

6a. Select some text, Ctrl-X, repeat N times
	Expected results:
	- The selected text blocks are cut
6b. Undo once
	Expected results:
	- The last cut operation was restored
	- The selection or caret position was restored
6c. Undo N-1 times
	Expected results:
	- The cut operations were restored in a step-by-step manner
	- The selections or caret positions in each step were restored

7. Repeat 6a-6c with Del and Ctrl-V

8a. Toolbar commands that change the document
	Procedure:
	- Select a toolbar command that changes the document (e.g. insert smiley)
	- Carry out the neccessary steps to make the change effective (e.g. choose smiley, ok)
	Expected results:
	- One undo step is generated after the change
	- Ctrl-Z or clicking "Undo" undoes the change
	- Ctrl-Y or clicking "Redo" after the previous point redoes the change
8b. Repeat the "procedure" part of 8a to repeat the same change to the document N times
	Expected results:
	- N undo steps are generated after the changes
	- Ctrl-Z/Undo button and Ctrl-Y/Redo button would undo/redo the changes in a stepwise manner