Undo should remember the cursor position

[jonhg]

When I have written a text and press undo the cursor is moved to the end of the document. This is not good when changing text inside a large article. The undo/redo system should remember the cursor position along with the snapshot.

Verified in IE7 nightly build (r598).

[jonhg]

Did not know that r598 linked to changeset 598 - meant nighly build built on revision 598 :)

[fredck]

Confirmed with IE7. Works well with Firefox.

[jonhg]

When I comment out the IE specific code in fckundo.js in (_GetBookmark and _SelectBookmark) and use the bookmark code which uses FCKDomRange the cursor position seems to be remember correctly.

I don't know the reason why this code isn't used today.

[martinkou]

Fixed with [615].

The undo system uses IE specific range and bookmark logic instead of the FCKDomRange wrapper because the wrapper would change the document temporarily in FCKDomRange.MoveToSelection(), which is called when the undo system is saving a snapshot. The temporary change by the wrapper would break some editor features (I don't remember exactly which, but I believe at least Cut is affected), so it isn't used.

As a side note, when I was doing my investigation before rewriting the undo system, I noticed an article by Mihai Bazon which might be useful for making FCKDomRange.MoveToSelection() less intrusive in IE. If we are able to reverse engineer the getBookmark() string in IE, then we can perform MoveToSelection() without writing markers to the document. Article here.

[jonhg]

I do not want to be difficult, but this still do not work.

1. Write a sentence. 2. Press enter a couple of times then write a new sentence 3. Go back under the first sentence and write a new one there.

4. Press undo - The caret is placed at the end of the "article", or I have also noticed: the caret is postioned in front of a sentence and seems to be jumping from that position to the end position every other undo.

[martinkou]

Your test case does not work because of an IE bug.

When you are pressing the Enter key in step 2, our current enter key component generates empty <p></p> tags in the document. These <p> tags do not have any child nodes when you examine them from the DOM viewpoint.

However, when you look at document.body.innerHTML, those <p> tags actually have an &nbsp; inside.

How is that relevant to the undo system? Well, when the undo system saves a snapshot in IE, and thus also saving the selection position, it generates a "bookmark" which remembers the selection position inside the document. When a bookmark is generated in step 2 or step 3 in IE, with the IE specific code, the bookmark's positions are based on the document with empty <p></p> tags, not <p>&nbps;</p> tags.

However, things are different when we restore the undo snapshot and the cursor position. Undo is done by restoring document.body.innerHTML in FCKeditor, so after the undo system restores the contents in the editing area, the previously empty <p></p> tags becomes <p>&nbsp;</p> tags - because of the IE bug I mentioned. Now, when the undo system attempts to restore the selection position as well, with IE's TextRange.moveToBookmark method: moveToBookmark() finds the bookmark's parent document is very different to the current document, and thus it would be meaningless to move the range there. So the selection position is not restored, and that's why you're seeing the caret going to the end of the document.

On the other hand, using the non-IE specific code does not work currently because FCKDomRange.MoveToSelection() and FCKDomRange.Select() change the document structure after they've executed. For example, they would split text nodes into parts.

I've tried various other methods today ( e.g. using document.body.cloneNode() to save undo snapshots instead of saving document.body.innerHTML ), but none of them worked. I haven't tried resorting to using the "dirtier" FCKDomRange.MoveToBookmark()/FCKDomRage.CreateBookmark() functions yet (they modify document.body.innerHTML themselves so they would create further complications to the undo system), but I remember they had their own problems when I was writing the undo system last month. If even the dirtier methods do not work well, then I'm afraid we'll have to rewrite fckdomrange_ie.js to make it less intrusive to the document structure.

[martinkou]

I've fixed the bug in jonhg's test case with [629]. Let's hope it won't cause other problems.