Re: [UndoManager] Disallowing live UndoManager on detached nodes

Hi folks,

I wanted to mention that, in addition to the extra implementation complexity, I am not sure that multiple independent UndoManagers per page is even a good feature.

The use cases document gives a use case of a text editor with an embedded vector graphics editor. But for all the native apps I know of that have this feature, they present a single unified undo queue per top level document, at least on the Mac.

Ryosuke also raised the possibility of multiple text fields having separate UndoManagers. On Mac, most apps wipe they undo queue when you change text field focus. WebKit preserves a single undo queue across text fields, so that tabbing out does not kill your ability to undo. I don't know of any app where you get separate switchable persistent undo queues. Thins are similar on iOS.

Maybe things are wildly different on Windows or Linux or Android. But my feeling is that the use case for scoped UndoManagers is dubious at best, and may cause authors to create confusing UI.

Since the use cases are dubious *and* this aspect of UndoManager causes extra implementation complexity, I think dropping it is the right thing to do.

Regards,
Maciej


On Aug 21, 2012, at 2:00 PM, Ryosuke Niwa <rniwa@webkit.org> wrote:

> Maciej, Ojan, and I had a further conversion about this matter off the list, and we've concluded that we should drop the support for undoscope content attribute altogether. So we're just going to do that and let authors use iframe to have multiple undo managers.
> 
> I can keep it around in the spec if other browser vendors are so inclined, but I'm somewhat skeptical that we can get the required two independent implementations given how much trouble we've had.
> 
> - Ryosuke
> 
> On Mon, Aug 20, 2012 at 9:52 PM, Ryosuke Niwa <rniwa@webkit.org> wrote:
> Greetings all,
> 
> We've been implementing undo manager in WebKit, and we've found out that allowing live undo manager on a detached undo scope host is a terrible idea.
> 
> e.g. say you have a subtree like follows:
> A
> B
> D
> C
> where A is the undo scope host. If we then detach B from A, and then insert A under D, all automatic transactions in A's undo manager are broken and may create a cyclic reference graph because nodes touched in automatic transactions need to be kept alive for undo/redo.
> 
> If there is no objection, I'm changing the spec to disallow live undo manager on detached nodes so that scripts can't move the host around inside a document or between documents; i.e. when a undo scope host is removed from its parent, its undo manager must be disconnected and a new undo manager be created for the node.
> 
> Alternatively, we can turn all automatic transactions in the undo manager into no-ops but I'd prefer disconnecting the undo manager altogether to make the behavior simple.
> 
> Best,
> Ryosuke Niwa
> Software Engineer
> Google Inc.
> 
> 
>