Hi Harbs, seems recent changes break Crux library (commit: f50c9990a3190cf681364905525656984ab2e9c5 - Cleaned up ElementWrapper and HTMLElementWrapper) I'm trying to see what could be the problem. I suppose that is the change of HTMLElementWrapper now extending ElementWrapper. Tried to change one for the other in Crux library, but with no luck I'm using /examples/crux/CruxQuickStartBasic to test Can you see what could be wrong? Thanks
Carlos El dom., 22 dic. 2019 a las 17:24, Harbs (<[email protected]>) escribió: > > There is lots of what looks like shared code, so could > HTMLElementWrapper extend ElementWrapper? > > Totally. Excellent idea. > > > On Dec 22, 2019, at 5:46 PM, Alex Harui <[email protected]> > wrote: > > > > In a quick look at history, HTMLElementWrapper's override logic was the > same as ElementWrapper's. > > > > Maybe as you upgraded HTMLElementWrapper's logic, ElementWrapper should > have changed as well but didn't? > > > > There is lots of what looks like shared code, so could > HTMLElementWrapper extend ElementWrapper? > > > > My 2 cents, > > -Alex > > > > On 12/22/19, 1:00 AM, "Harbs" <[email protected]> wrote: > > > > We found a weird bug with events and currentTarget. > > > > I traced the problem to the following: > > > > The app loads both HTMLElementWrapper and ElementWrapper. The lstener > overrides in the two are stepping on each other. Here’s what happens: > > > > 1. HTMLElementWrapper is loaded first. It replaces > goog.events.fireListener with its fireListenerOverride function (which > calls the existing one when it’s done). > > 2. ElementWrapper is loaded next and it replaces the existing > goog.events.fireListener function — which was already changed to point to > HTMLElementWrapper.fireListenerOverride with the one from ElementWrapper. > > 3. When an event is actually dispatched, > ElementWrapper.fireListenerOverride first changes the event to a royale > BrowserEvent instead of a goog one. HTMLElementWrapper.fireListenerOverride > is then called and where it expects a goog BrowserEvent, it in fact gets a > royale BrowserEvent. This causes the wrappedEvent to be the wrong type and > messes things up down the line. > > > > I’m not sure of the best way to fix this. > > > > * We could check the event type in HTMLElementWrapper/ElementWrapper, > but that’s just-in-case code. > > * I’m not completely sure why we need this logic in both > ElementWrapper and HTMLElementWrapper. Is there something that can be > changed there? > > * Maybe there’s some way for ElementWrapper to know that some other > class is installing an override? > > > > Thoughts? > > Harbs > > > > -- Carlos Rovira http://about.me/carlosrovira
