Nevermind, didn't see the PR. Thanks for creating it. Looking into it. On 8/31/16, 9:23 PM, "Alex Harui" <aha...@adobe.com> wrote:
>Before we try to patch Closure Library, I would still like to examine a >simple test case. I think it would help me understand the problem. I >thought that folks who created custom IEventDispatchers without extending >EventDispatcher wrapped an EventDispatcher and passed in a reference to >the instance to that EventDispatcher. Are you saying it didn't work at >all in JS? Or just in some scenario? > >Thanks, >-Alex > >On 8/31/16, 4:47 PM, "Greg Dove" <greg.d...@gmail.com> wrote: > >>"I guess we need something on goog's EventTarget that is more >>setExternalTarget and that is only used for the event.target , not for >>the >>fireListeners call. " >> >>This type of change to their eventtarget.js works [1] and all that is >>needed is a change to setTargetForTesting(target) to be >>setTargetForTesting(target,true) in our constructor. Then that hack I >>added >>is no longer needed. >> >>I have not issued any request against closure lib there yet, but can do >>so >>if you think it is best (I don't know how it will be received). I'm not >>entirely comfortable doing this on behalf of Apache Flex, so would prefer >>if someone on the team did it. >> >>I can't see another simpler way around this (which doesn't mean there >>isn't >>one!). I definitely didn't want to put that method in the interface >>because >>it would deviate from flash. >> >> >> >>1. >>https://github.com/greg-dove/closure-library/commit/6be3161f02cf327a0dc1a >>3 >>3f085f0531d2993b4e >> >> >> >>On Thu, Sep 1, 2016 at 10:03 AM, Greg Dove <greg.d...@gmail.com> wrote: >> >>> Alex, I just issued a PR that lets you easily see the problem (along >>>with >>> a fix for ant script for the manual test). >>> >>> I had a quick look inside eventtarget.js and it seems pretty clear to >>>me: >>> >>> >>>https://github.com/google/closure-library/blob/master/closure/goog/event >>>s >>>/ >>> eventtarget.js#L349 >>> >>> it is calling currentTarget.fireListeners and in the most basic case >>>for >>> proxy event dispatching, this is the same as the 'targetForTesting' >>>which >>> requires that fireListeners is implemented on the alternate target. >>> when a subclass inherits EventDispatcher, currentTarget will always >>> be 'this' and so the same instance EventTarget's fireListeners method >>>is >>> called directly. >>> >>> all the other variables and functions are referenced via 'this' which >>> would correctly resolve inside the proxy EventDispatcher instance, >>>although >>> they would all obviously be inherited if the class extends the flexjs >>> EventDispatcher. >>> >>> I guess we need something on goog's EventTarget that is more >>> setExternalTarget and that is only used for the event.target , not for >>>the >>> fireListeners call. >>> >>> >>> >>> On Thu, Sep 1, 2016 at 3:51 AM, Alex Harui <aha...@adobe.com> wrote: >>> >>>> >>>> >>>> On 8/31/16, 12:43 AM, "Greg Dove" <greg.d...@gmail.com> wrote: >>>> >>>> >I observed that issue when implementing IEventdispatcher i.e. when >>>>the >>>> >EventDispatcher constructor has an IEventDispatcher argument - so not >>>>for >>>> >statics. This is not seen when extending EventDispatcher, but in that >>>> case >>>> >the subclass would presumably inherit that method from goog >>>>EventTarget. >>>> >So >>>> >that setCurrentTarget method in the constructor did not seem to be >>>>the >>>> >complete answer here... that extra hack in the constructor just got >>>> things >>>> >working for now....it works as is but I don't like it. >>>> >>>> Interesting. Do you have a simple test case we can look at? Looking >>>>at >>>> the EventTarget code, I would think there would be other functions and >>>> variables that need propagation. >>>> >>>> Thanks, >>>> -Alex >>>> >>>> >>> >
