Undo to the wrong state

[circleb]

There appears to be a bug with the contenteditable demo, here's how it goes:

1. Highlight any word and click the bold button
2. Click undo
3. Highlight another word and make it bold
4. Click undo
5. Now the editable div goes to back to the state between 1 & 2, basically it should go back to nothing being highlighted, but instead it goes back to the state when the first word was highlighted.

I've tried hitting save at all of the various intervals in the sequence but this is very persistent...

[jzaefferer]

Thanks for the report. Though since this is just a demo, I don't have plans for fixing this issue. I'd review a PR and merge it, but that's about it.

[circleb]

Ok, thanks for the reply, I'm also reviewing Arthur's Undo Manager. If that ends up being a dead end, I may see what I can do as far as fixing yours.

[jzaefferer]

If you find something that works better than this proof of concept, I'd be happy to point everyone to that instead (in the readme).

[hanstiono]

@jzaefferer This is a very nice undo plugin. but there is still a bug just like what @circleb told.
After doing some action and I tried to undo the first time, the "stackPosition" didn't went down by one (should be stackPosition--) but instead it increased by one (stackPosition++) and the number of array in "command" went up (the array number should be the same). The problem came when I tried to redo it. The state is quite messy after that
I just realized that after implementing quite a lot of code with this plugin.. that's too bad

[jzaefferer]

Sorry to hear that, but my previous comments still apply. PRs are welcome, but I can't help beyond that.

[hanstiono]

Yeah, that's fine. I'll try to take a look at your code later.
Anyway thank you for making it

[BeginnerGitHub]

Hi,

I tested this plugin and I also discovered a bug in the demo, of the same type as the one explained by circleb.

The problem comes from the variable "Startvalue": this variable is updated when the text changes but it is not updated when you click on the "Undo" button ...

So after a "undo" the variable "startvalue" is not updated so when you modify the text you record in the undo stack an "EditCommand" with a bad value for the property "oldValue".

1- You can solve the first problem in several ways, here is a way:
Replace this code:

$(".bold").click(function() {
	document.execCommand("bold", false);
	var newValue = text.html(); 
	stack.execute(new EditCommand(text, startValue, newValue));
	startValue = newValue;
});

by :

$(".bold").click(function () {
    document.execCommand("bold", false);
    var newValue = text.html();
    startValue = stack.commands[stack.stackPosition]?.newValue || startValue
    stack.execute(new EditCommand(text, startValue, newValue));
    //startValue = newValue;        
});

2- You can solve the second problem by replacing this code:

$("#text").bind("keyup", function() {
	// a way too simple algorithm in place of single-character undo
	clearTimeout(timer);
	timer = setTimeout(function() {
		var newValue = text.html();
		// ignore meta key presses
		if (newValue != startValue) {
			// this could try and make a diff instead of storing snapshots
			stack.execute(new EditCommand(text, startValue, newValue));
			startValue = newValue;
		}
	}, 250);
});

by :

$("#text").bind("keyup", function () {
    // a way too simple algorithm in place of single-character undo
    clearTimeout(timer);
    timer = setTimeout(function () {
        var newValue = text.html();  

        startValue = stack.commands[stack.stackPosition]?.newValue || startValue // ajout
        // ignore meta key presses
        if (newValue != startValue) {
            // this could try and make a diff instead of storing snapshots
            stack.execute(new EditCommand(text, startValue, newValue));           
            //startValue = newValue; // modif
            
        }
    }, 250);
});