[whatwg] Feedback on UndoManager spec

Ryosuke Niwa rniwa at webkit.org
Wed Oct 26 11:39:06 PDT 2011

On Wed, Oct 26, 2011 at 10:57 AM, Aryeh Gregor <ayg at aryeh.name> wrote:

> >> 2) What happens if you have an Element with the undoscope attribute
>  >> that doesn't descend from a Document?  Does undo management make any
> >> sense in that case, or should the undoscope attribute have no effect
> >> for detached elements?
> >
> > This is a debatable point. On one hand, allowing a node with undoManager
> to
> > be moved to another location in DOM seems nice but on the other hand,
> being
> > able to move a node with undoManager to a different document will be
> > problematic. And semantically, moving undoManager makes very little
> sense.
> Well, if you do
>  var span = document.createElement("span");
>  span.undoScope = true;
> what is the value of span.undoManager?  Per the current spec, it
> should return an UndoManager that works just fine.  Is this desired,
> or do you want to return null in this case?

I'm not sure if we should allow that or not. I'm inclined towards not
allowing it for the sake of disallowing undoManager on a detached node.

> I'm thinking whether DOM state should also include properties on the node
> or
> > not.
> You mean IDL properties?  We can't include all of those in DOM state.
> For instance, it would be extremely unexpected if undo were to undo
> changes to document.cookie.  Or did you just mean custom IDL
> properties that the author added via script, not built-in IDL
> properties?

I meant properties authors added to nodes. e.g.
span.myProperty = true;

Should span be removed by some automatic transaction, authors may expect it
to be restored on undo.

>> 11) "Any changes made to the value of the isAutomatic attribute after
> >> the transaction had been applied should not change the type of the
> >> transaction."  What about changing other things about it?  If I do
> >>
> >>  var transaction = { label: "x", apply: foo, unapply: bar, reapply:
> >> baz, isAutomatic: false };
> >>  document.undoManager.transact(transaction);
> >>  transaction.unapply = quz;
> >>  document.undoManager.undo();
> >>
> >> which function is called, bar or quz?
> >
> > quz.
> This should be spelled out explicitly, IMO.

The assumption is transaction works like a regular object unless otherwise
stated. I guess I can cite your example though.

 > No. I did not specify that because the only requirement is that UAs
> restore
> > DOM states.
> > I specifically avoided to give any guarantee or implication as to in what
> > order things are restored
> > to allow optimizations.
> But this doesn't define what happens in the face of manual
> transactions.  Also, it's not precise even if there are no manual
> transactions.

UAs don't do anything for manual transactions. They just call

If a node is removed from the DOM and undoing restores
> it, does it restore the same object or a copy?  If a copy, does it
> include custom properties that the author added or not?  I suspect
> you'll say that this is deliberately undefined for the sake of
> performance, but it's a potential interop issue.

This is well defined in terms of DOM state. The spec says UAs should restore
the DOM state
so it all depends on how DOM state is defined.

I also vaguely remember Ehsan telling me Gecko might have a trouble
restoring the exactly same object
on undo/redo because of the way its undo and redo are implemented.

>> 15) Is the isReapply parameter to apply() needed?  The only place I
> >> see where it's used is if the author specifies a manual transaction
> >> but leaves off a reapply() method.  In that case, why not just call
> >> apply() with no extra parameter?  If the author wanted to have apply()
> >> and reapply() behave differently, they could have specified a separate
> >> reapply() method.
> >
> > There are good arguments made by Jonas on this topic.
> > Please look at whatwg archives on this topic.
> I looked at the archives and didn't see any good arguments.  As far as
> I can tell, if authors wanted behavior like with the isReapply
> parameter, they could easily emulate it by changing
>  { apply: f }
> to
>  { apply: function() { f(true) }, reapply: function() { f(false) } }

The same is true for having apply and reapply. Jonas wanted to get rid of
reapply altogether and just have
void apply(in boolean isReapply)

In this world, you could do
{ apply: function(isReapply) { return isReapply ? this.doApply() :
this.doReapply(); } }.

I didn't want this API because I'd expect apply and reapply to be
substantially different.

- Ryosuke

More information about the whatwg mailing list