a code review -- issue-734-double-events-r618.patch

[Joel Webber]

Hello Kelly and Rajeev,

I'd like you to do a code review.  To review this change please execute in the trunk:

       svn update -r618
       patch -p0 < issue-734-double-events-r618.patch

to review the following code:

This is to fix issue 734: http://code.google.com/p/google-web-toolkit/issues/detail?id=734.

The basic issue stems from *really old* code in DOMImpl[Standard|IE6] that used to simulate something like event bubbling. In a nutshell, the event dispatcher used to crawl up the DOM, looking for the first element with a listener. Most of the time, this has no effect, but it can lead to multiple firings of the same event when it is allowed to bubble up.

This should never have persisted this long, and I've found that fixing it (which also simplifies the code) gets rid of the double firing problem, and all of our samples still work just fine.

It did uncover a bug in Menus, though, and the fix should be self explanatory.

[Kelly Norton]

LGTM.

What platforms have you run tests on? I know you have to fly soon, so I'd be happy to fire off test runs for any untested platforms and commit while you are in the air.

/kel

[Joel Webber]

It seemed to work properly on Safari, FF, and IE. It's probably worth checking it out on Linux hosted mode (crappy old gecko), just to be sure, though I doubt it will be a problem.

The only funky part was that it exposed a bug in Menu[Item], but the old code was simply wrong. It didn't affect anything else I tested.

Thanks for doing this!
joel.

--
joel.

[Kelly Norton]

Committed as r670 after running tests on all platforms.
/kel

[Rajeev Dayal]

Late LGTM after Kelly's help with understanding some of the event dispatching infrastructure.