replace undo/redo with events that only change DOM?

[johanneswilm]

@ojanvafai wrote on August 18, 2016 23:59 in #18

I wonder if a similar logic would apply to undo/redo, i.e. make preventDefault not prevent the items from getting popped from the undo stack? The complicated thing there is coming up with the name since undo/redo can both do deletes and insert. undoModification and redoModification maybe? That's wordy, but I think it at least would make it clear that the event is for the DOM modification, not the other parts of the user action.

[johanneswilm]

I wonder: It seems that if there is any programmatic change to the DOM inside the editing host, the undo/redo history stops making sense. There may be cases where it happens to still work, but there is no guarantee for it working, so the JS will need to maintain its history anyway. I wonder if we should specify that doing any change to the DOM within the editor by means of JS or preventDefaulting a beforeinput event will reset the history.

Or can someone think of a usecase for the browser undo history when the JS changes the DOM inside the editor?

[johanneswilm]

@choniong Question: When I preventDefault a beforeinput event, will it still create an entry for the undo stack? And if it does, what information goes in there? Is it the operation that would have occurred had we not prevented it?

@ojanvafai I think your proposal made sense at the time, given what we knew back then. Now that we know that the undo stack is permanently broken when contenteditable is involved, I am not so sure it still makes as much sense.

If the editor handling a beforeinput event means that nothing is added to the undo stack, then it would seem odd that something is still removed from the stack when undo/redo are handled.

If, on the other hand, we say that beforeinput continues to create undo stack entries although it is being handled in JS, then we still need to create a separate undo stack in JS because the default global undo stack won't know what we really changed in the DOM, and undo will be broken for other elements on the page because the undo stack is filled with items from the contenteditable element.

Let's also not forget that adding completely new items, such as adding images through a editor-provided user interface won't cause any beforeinput event, so therefore it won't add anything to the undo stack. Yet the user still requires it to be reverted when hitting the undo shortcut. So then it the global undo stack and the one maintained by JS will be out of sync.

What usecase is there for the behavior where the DOM is not modified yet the last operation is popped from the undostack? Doesn't that just mean that one can be 100% certain that the undo stack will be broken?

Also, in the current situation where we cannot enable the undo/redo menus permanently, it means we will have to execute the hack that adds elements to the undo stack with every single handling of the beforeinput event so that the menus stay active.

[chong-z]

Question: When I preventDefault a beforeinput event, will it still create an entry for the undo stack?

No.

If the editor handling a beforeinput event means that nothing is added to the undo stack, then it would seem odd that something is still removed from the stack when undo/redo are handled.

Makes sense.

I think the originally idea is: InputEvent and UndoMananger/Undo Stack/Undo Events are different area, and we shouldn't mix them together.

So for InputEvent we just want to be able to prevent modification to an element, regardless how undo stack works. For example

- Imaging I drag&drop text from editing host A to B, and hit `undo` when the focus is on C

Since A&B are going to be modified by the `undo` command, they should be expecting 'beforeinput' (1 for A, 1 for B) and should be able to cancel the modification.

But what should happen if we decided to be able to prevent popping undo stack?

Or should we actually just want to send one beforeinput to document?

[johanneswilm]

I think the originally idea is: InputEvent and UndoMananger/Undo Stack/Undo Events are different area, and we shouldn't mix them together.

Yes, i think the idea was to decouple beforeinput canceling from some of the other things that happen, such as the clipboard changing, etc. . In order to stay consist, we tried to extend this to the undo/redo events. But in the end it turns out it actually makes things less consistent that way.

• Imaging I drag&drop text from editing host A to B, and hit undo when the focus is on C Since A&B are going to be modified by the undo command, they should be expecting 'beforeinput' (1 for A, 1 for B) and should be able to cancel the modification.

They both need to receive a beforeinput event. I don't think it should be sent to document, because that only means that these webapps aren't sure where exactly they need to listen to things.The event should just be sent to the element where the focus currently is, not where the last undo stack item was moved in.

I think the question about what happens to the global undo stack turns out to be a little "academic" because in reality every contenteditable editor we have been able to find has implemented their own undo stack, and the defacto standard is for every richtext element to have their own undo stack. So that means if you undo in the element was dragged from, it gets back there (so it's in the document twice), and if you undo in the one it was dragged to, it will be removed there and not be visible anywhere.

So when things break in textareas and input fields anyway, it's because the person creating the site hasn't realized that officially there is something called a "global undo stack" that one needs to program around somehow.

The problem will eventually be solved once we either get an undo manager that permits setting the undo stack to be limited to one element, or once richtext elements are removed from the global undo stack by default. Until that time, the undo stack continues to be broken.

[johanneswilm]

If you are 100% tied to the model with the global undo stack, I agree with your suggestion to always fire beforeinput for undo/redo only on the document. That way JS developers can listen there and check if the selection is in their element, and if that is the case, they can preventDefault and handle it themselves. This would be slightly odd, but at least it would mean they could use the signal instead of listening for keystrokes.

[chong-z]

They both need to receive a beforeinput event.

The event should just be sent to the element where the focus currently is, not where the last undo stack item was moved in.

Sorry if I misunderstood, but the above two statements seems contradicted to me?

Yes I understand the undo stack is broken and it's useless to ContentEditable editors, and we need a better solution for it, but for this issue I guess we should as least make the event consistent by itself?

So back to my example, assuming A,B,C are just UA default ContentEditable, by hitting undo when focusing C, one of the following could happen:

1. Only C (focused element) receives beforeinput-undo, default actions is to modify A,B
   • preventDefault() will cancel modification on both A and B
   • preventDefault() prevents popping stack (or we could make it not)
2. Only A,B (elements to be modified) receive beforeinput-undo
   • Preventing default on A will only cancel modification to A
   • Undo stack always pops
3. Only A,B (elements to be modified) receive beforeinput-undo
   • Preventing at least one beforeinput will cancel modification on both A and B
   • Undo stack won't pop with at least one preventDefault()
4. Fire on document

If I understand correctly you are suggesting 1? I'm happen with 1 as it's the current implementation (the easiest approach), but I just want to confirm:

• Is it OK for A and B to be modified without receiving beforeinput?

(I know in practice there won't be undo entries for editors, but I think we still need to make the default cE work)

[johanneswilm]

Sorry if I misunderstood, but the above two statements seems contradicted to me?

I can see how one might read them that way. Let me try again:

All actions, except undo and redo, the beforeinput event should go the element where the change occurs. So in the case of one element being dragged from one element to another, the first element should receive the beforeinput event for drag (dragend) and the second should receive the one for drop,

Undo/Redo

These are special, because there seem to be two "standards" here: The browsers seem to operate with a "global undo stack". All the websites with richtext editing that we have been able to find thus far follow a model where undo stacks are per element and not global, including the sites by all the main browser makers.

As long as we have two different models about where the change is to happen, we will run into the problem that the beforeinput event is triggered on a different element than what JS expects and needs. For this reason, I would have a prioritized list of what should be done with the undo event:

1. The best would be if the browser dropped its global undo stack for contenteditable elements, realizing that noone uses it and it only breaks stuff. The beforeinput event for undo/redo would then always and only be triggered on the element with focus. This option currently seems unfeasible, given that at least the Safari team sees this as being an independent UA-decision.

2. The second best solution seems to be that all beforeinput events for undo/redo always and only are triggered on the document. That way the the JS editor only has to listen for beforeinput events in not more than two places (the document and the element that is being edited on). And it's relatively straightforward to figure out which undo/redo events need to be intercepted for just this element (every time the focus is on the editor element), and if all beforeinput events for a particular contenteditable element are handled it shouldn't even destroy the global undo stack (only problem then is how to keep the undo/redo menu times active).

   More than one undo

Not sure if this was also something you thought, that the drag and drop, given that it consists of two partial operations needs to trigger two beforeinput undo events when someone tries to undo the operation after the drag-dropping operation has finished. This I would not think makes sense. if a user hits the undo keyboard shortcut once, there should only be one beforeinput undo event. This may have some funny results, such as:

1. User drags element from editable element A to B. Both elements receive one beforeinput event: A for the drag, B for the drop.
2. Focus remains in element B after the drop.
3. User hits undo, which is being triggered on the document.
4. Both the editor for A and for B listen to the undo event on the document, but A's editor ignores it, as it notices that the selection isn't in A. B's editor sees that B has the focus, so it handles it.
5. Result: The element disappears from B, but isn't added back to A.

While slightly funny, I think this is acceptable. If the person maintaining the website really wanted A and B to be fully integrated, she will have to write some extra code to make them stick better together.

Does that make sense?

[johanneswilm]

@choniong

I now realize that triggering beforeinput for undo/redo on the document is not very helpful if you want to implement a editing system with several richtext editing surfaces and you want to follow the global-undo stack model.

So how about a third option:

3 . Trigger all beforeinput events for undo redo on the document and point their target range at the editing host the browser thinks they should undo/redo. That way the JS editor developers should have all the information that is needed:

a. If the JS editor wants to have one undo stack per editing host and controls all the editing hostssurfaces, the editor ignores the target ranges for and only looks at the selection whenever the undo beforeinput event is triggered.

b. If the JS editor wants to have one global undo stack for all editing hosts and controls all of the editing surfaces, the editor ignores all target ranges and the selection, and maintains one global undo stack in Javascript.

c. If the JS editor wants to have one undo stack per editing host, but doesn't control all the editing hosts, the editor looks at the target ranges of the beforeinput events, and if it points as her editing host, she lets the event through and then reverts the DOM change when the input event is triggered. She lets all other undo events through as well, with the exception of when the selection is in her editing host. If the selection is in her editing host, she cancels the event and handles it herself.

d. If the JS editor wants to have a global stack, but he doesn't control all the editing hosts, the editor looks at the target ranges of the beforeinput events, and if it points at his editing host, he lets the event through and then reverts the DOM change when the input event is triggered, and then applies the change himself in JS.

This is all quite complicated because of the global undo stack. Maybe another option could be to come up with a simple way of disabling the global undo stack for richtext editing before we get the new element?

[chong-z]

she lets the event through and then reverts the DOM change when the input event is triggered

So JS is actually just trying to insert a dummy undo entry by letting events through and revert? I believe JS can achieve this quite easily with UndoManager.

---

Since there is no easy good way to solve the undo stack issue, I think we should stick at describing UA's behavior.

Proposal How about thinking in this way:

• 'beforeinput' for Undo/Redo should be fired on undoscope of focus, and preventing default will prevent popping undo stack

Background Concept undoscope is borrowed from UndoManager, where each undoscope has an UndoManager instance managing an undo stack.

And by default document is an undoscope.

[johanneswilm]

So JS is actually just trying to insert a dummy undo entry by letting events through and revert? I believe JS can achieve this quite easily with UndoManager.

Thinking more about it, if JS wants to integrate with the global undo stack, it will likely have to add dummy entries for every editing operation. And then it won't have to do much of anything to make sure that they don't actually affect the DOM.

• 'beforeinput' for Undo/Redo should be fired on undoscope of focus, and preventing default will prevent popping undo stack

I think that is a good line of argument. So basically we go for option 2: beforeinput for all editing operations on the editing host, except redo/undo which will be on the document until all UAs have implemented undo managers.

In practice that means that this will work well for those who want to have a separate undo stack per element and not as helpful for those who want to have a global undo stack as long as there is no good undo manager. For me this definitely OK, and existing editors would be able to integrate with the native controls somewhat better without changing the way they work.