Bug 5874 – WebCore+SVG needs support for "evt" instead of event

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Eric Seidel (no email)]

For whatever reason the SVG working group chose to specify "evt" as the name of the parameter passed to 
inline event handlers.

This simple patch adds support for this to WebCore.

[Eric Seidel (no email)]

Created attachment 4844 [details]
simple patch adding "evt" support

[Eric Seidel (no email)]

Comment on attachment 4844 [details]
simple patch adding "evt" support

mjs, darin or ggaren could easily review this.

[Darin Adler]

Comment on attachment 4844 [details]
simple patch adding "evt" support

I think a cleaner and clearer way to do this is to have a createSVGEventHandler
alternative to the createHTMLEventHandler function that follows the SVG rules
instead.

To control the parameter name used when constructing the event listener, I
suggest either adding a parameter to the JSLazyEventListener constructor or
creating derived classes specific to HTML and SVG.

[Maciej Stachowiak]

Agreed, there should be a different code path for SVG rather than testing the element like this.

[Dave Hyatt]

I believe you should support both evt and event.  I think that's something I recall was decided (either by 
Mozilla or CDF or someone).

[Eric Seidel (no email)]

Created attachment 5836 [details]
addressed Darin's concerns

This new patch adds an JSSVGLazyEventListener subclass for SVG to use.  This patch also simplifies the way that event listeners are created by removing two unecessary layers of abstraction.  This should address both darin and maciej's concerns.  We do not currently support both "evt" and "event" for SVG event handlers like hyatt (and others) suggested.  That dual functionality can be tracked by a separate bug.

[Darin Adler]

Comment on attachment 5836 [details]
addressed Darin's concerns

I don't like the underscore-prefixed name for the local variable. We should steer away from underscore prefixes in general since they are reserved for the library and platform.

Patch does not include JSSVGLazyEventListener.h and .cpp so I couldn't review them. Because of that, review-.

[Eric Seidel (no email)]

Created attachment 5851 [details]
addressed darin's further concerns

[Darin Adler]

Comment on attachment 5851 [details]
addressed darin's further concerns

I made a couple of comments on IRC. r=me (with or without the refinements from IRC).