Summary: | WebCore+SVG needs support for "evt" instead of event | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||
Component: | SVG | Assignee: | 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
Eric Seidel (no email)
2005-11-29 03:48:31 PST
Created attachment 4844 [details]
simple patch adding "evt" support
Comment on attachment 4844 [details]
simple patch adding "evt" support
mjs, darin or ggaren could easily review this.
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.
Agreed, there should be a different code path for SVG rather than testing the element like this. 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). 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 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-.
Created attachment 5851 [details]
addressed darin's further concerns
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).
|