Re: [UndoManager] Re-introduce DOMTransaction interface?

But anyhow, event based API is ok to me.
In general I prefer events/event listeners over other callbacks.


On 07/05/2012 11:37 AM, Olli Pettay wrote:
> On 07/05/2012 08:00 AM, Adam Barth wrote:
>> On Wed, Jul 4, 2012 at 5:25 PM, Olli Pettay <Olli.Pettay@helsinki.fi> wrote:
>>> On 07/05/2012 03:11 AM, Ryosuke Niwa wrote:
>>>>
>>>> On Wed, Jul 4, 2012 at 5:00 PM, Olli Pettay <Olli.Pettay@helsinki.fi
>>>> <mailto:Olli.Pettay@helsinki.fi>> wrote:
>>>>
>>>>      On 07/05/2012 01:38 AM, Ryosuke Niwa wrote:
>>>>
>>>>          Hi all,
>>>>
>>>>          Sukolsak has been implementing the Undo Manager API in WebKit but
>>>> the fact undoManager.transact() takes a pure JS object with callback
>>>>          functions is
>>>>          making it very challenging.  The problem is that this object needs
>>>> to be kept alive by either JS reference or DOM but doesn't have a backing
>>>> C++
>>>>          object.  Also, as far as we've looked, there are no other
>>>> specification that uses the same mechanism.
>>>>
>>>>
>>>>      I don't understand what is difficult.
>>>>      How is that any different to
>>>>      target.addEventListener("foo", { handleEvent: function() {}})
>>>>
>>>>
>>>> It will be very similar to that except this object is going to have 3
>>>> callbacks instead of one.
>>>>
>>>> The problem is that the event listener is a very special object in WebKit
>>>> for which we have a lot of custom binding code. We don't want to implement a
>>>> similar behavior for the DOM transaction because it's very error prone.
>>>
>>>
>>> So, it is very much implementation detail.
>>> (And I still don't understand how a callback can be so hard in this case.
>>> There are plenty of different kinds of callback objects.
>>>   new MutationObserver(some_callback_function_object) )
>>
>> I haven't tested, by my reading of the MutationObserver implementation
>> in WebKit is that it leaks.  Specifically:
>>
>> MutationObserver --retains--> MutationCallback --retains-->
>> some_callback_function_object --retains--> MutationObserver
>>
>> I don't see any code that breaks this cycle.
>
>
> Ok. In Gecko cycle collector breaks the cycle. But very much an implementation detail.
>
>
>>
>> DOM events
> Probably EventListeners, not Events.
>
>> have a bunch of delicate code to avoid break these
>> reference cycles and avoid leaks.  We can re-invent that wheel here,
> Or use some generic approach to fix such leaks.
>
>> but it's going to be buggy and leaky.
> In certain kinds of implementations.
>
>>
>> I appreciatie that these jQuery-style APIs are fashionable at the
>> moment, but API fashions come and go.  If we use this approach, we'll
>> need to maintain this buggy, leaky code forever.
> Implementation detail. Very much so :)
>
> Do JS callbacks cause implementation problems in Presto or Trident?
>
>
>
>
> -Olli
>
>
>
>> Instead, we can save
>> ourselves a lot of pain by just using events, like the rest of the web
>> platform.
>>
>> Adam
>>
>>
>>>>          Since I want to make the API consistent with the rest of the
>>>> platform and the implementation maintainable in WebKit, I propose the
>>>> following
>>>>          changes:
>>>>
>>>>             * Re-introduce DOMTransaction interface so that scripts can
>>>> instantiate new DOMTransaction().
>>>>             * Introduce AutomaticDOMTransaction that inherits from
>>>> DOMTransaction and has a constructor that takes two arguments: a function
>>>> and an
>>>>          optional label
>>>>
>>>>
>>>>          After this change, authors can write:
>>>>          scope.undoManager.transact(new AutomaticDOMTransaction{__function
>>>> () {
>>>>
>>>>                scope.appendChild("foo");
>>>>          }, 'append "foo"'));
>>>>
>>>>
>>>>      Looks somewhat odd. DOMTransaction would be just a container for a
>>>> callback?
>>>>
>>>>
>>>> Right. If we wanted, we can make DOMTransaction an event target and
>>>> implement execute, undo, & redo as event listeners to further simplify the
>>>> matter.
>>>
>>>
>>> That could make the code more consistent with rest of the platform, but the
>>> API would become harder to use.
>>>
>>>>
>>>> - Ryosuke
>>>>
>>>
>
>

Received on Thursday, 5 July 2012 09:23:44 UTC