Yeah, I guess that is not 'simple' in terms of an example but it was easier to show it with what was already there.
"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." yep, I am definitely one of those folks too :). That's what I am doing. "Are you saying it didn't work atall in JS?" I did not see it work, and based on the eventtarget code, I can't see how it can. I don't actually know what the intent of their setTargetForTesting is so I am not sure if it is a bug or whether they only ever intended one EventTarget to be manipulating another. The typing in their code is Object, not goog.events.EventTarget for that method's argument, so it might even be a bug on their side. I made another minor change to what I had against eventtarget, which avoids a little branching - if you replace your library copy of eventtarget.js with this [1] you should be able to change the setTargetForTesting call to setTargetForTesting(target,true) and the hack can be avoided. 1. https://raw.githubusercontent.com/greg-dove/closure-library/342fa762984b10e53ebe9fcb906b7af813edb739/closure/goog/events/eventtarget.js On Thu, Sep 1, 2016 at 4:41 PM, Alex Harui <aha...@adobe.com> wrote: > 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 > >>>> > >>>> > >>> > > > >
