Bug 5874

Summary: WebCore+SVG needs support for "evt" instead of event
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: SVGAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P4    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Bug Depends on:    
Bug Blocks: 6026    
Attachments:
Description Flags
simple patch adding "evt" support
darin: review-
addressed Darin's concerns
darin: review-
addressed darin's further concerns darin: review+

Description Eric Seidel (no email) 2005-11-29 03:48:31 PST
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.
Comment 1 Eric Seidel (no email) 2005-11-29 03:48:49 PST
Created attachment 4844 [details]
simple patch adding "evt" support
Comment 2 Eric Seidel (no email) 2005-11-29 03:49:08 PST
Comment on attachment 4844 [details]
simple patch adding "evt" support

mjs, darin or ggaren could easily review this.
Comment 3 Darin Adler 2005-11-29 07:29:19 PST
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.
Comment 4 Maciej Stachowiak 2005-11-29 15:38:47 PST
Agreed, there should be a different code path for SVG rather than testing the element like this.
Comment 5 Dave Hyatt 2006-01-15 00:21:17 PST
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).
Comment 6 Eric Seidel (no email) 2006-01-22 04:40:26 PST
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.
Comment 7 Darin Adler 2006-01-22 10:46:20 PST
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-.
Comment 8 Eric Seidel (no email) 2006-01-22 15:18:07 PST
Created attachment 5851 [details]
addressed darin's further concerns
Comment 9 Darin Adler 2006-01-22 15:23:03 PST
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).