Side navigation
#13180 closed bug (fixed)
Opened January 08, 2013 07:16PM UTC
Closed January 14, 2013 12:22AM UTC
Last modified January 16, 2013 03:07AM UTC
Delegated event not firing for click within SVG use element
Reported by: | dankellett | Owned by: | gibson042 |
---|---|---|---|
Priority: | low | Milestone: | 1.9 |
Component: | event | Version: | 1.8.1 |
Keywords: | Cc: | gibson042, timmywil | |
Blocked by: | Blocking: |
Description
I have created a jsbin illustrating the potential bug.
http://jsbin.com/welcome/71517/
The first js statement attaches an event using the 'live' functionality of the .on() method. This event does not fire when you click on the chart image, but will fire when you click on the SVG outside of the graph image. The event will write 'live click' to the console.
The second statement attaches an event using a regular direct event attachment to the svg container. This does work when you click on the chart and elsewhere in the SVG. The event will write 'on click' to the console.
I would expect that the two event handlers would fire no matter where you click within the SVG. The difference being that the 'live' type attachment would allow me to detach and reattach a new SVG target to the DOM without having to reattach the event handler.
NOTE: the jsbin is using version 1.8.1, if a later version is used, a different javascript error is thrown: "Object #<SVGElementInstance> has no method 'getAttribute"
Attachments (0)
Change History (12)
Changed January 08, 2013 07:22PM UTC by comment:1
_comment0: | From my tests, this issue occurs in Chrome and IE9, but not FF. \ \ Also, I attempted to use jsfiddle instead of jsbin, but I could not get the SVG to render in jsfiddle. → 1357673203647813 |
---|---|
_comment1: | From my tests, this issue occurs in Chrome and IE9, but not FF. \ \ Regarding the guidelines to use jsfiddle; I attempted to use jsfiddle instead of jsbin, but I could not get the SVG to render in jsfiddle. → 1357673289034462 |
Changed January 08, 2013 07:52PM UTC by comment:2
cc: | → gibson042, timmywil |
---|---|
component: | unfiled → event |
milestone: | None → 1.9 |
priority: | undecided → low |
status: | new → open |
Thanks for the clear explanation and test case!
Delegated events have never worked on SVG elements since they have no DOM methods we can use. I opened a documentation ticket for that. https://github.com/jquery/api.jquery.com/issues/211
In 1.8+ jQuery throws an error on this case in Sizzle, but prior to that it silently ignores delegate clicks within SVG elements. I think we should do the latter, both for backcompat and because delegated event handlers have no way of protecting themselves against events that bubble up from within SVG compartments. @gibson042 or @timmywil?
Changed January 10, 2013 10:29PM UTC by comment:3
Here's what's going on:
1. For delegated events, jQuery.event.dispatch
checks every element from the event target up to the delegate to see if one matches the delegation selector.
2. When possible, that check takes a fast approach that passes a seed set to Sizzle
3. Sizzle runs Expr.filter
matchers against the seed, which in many cases (including this one) assume the availability of .getAttribute
or other DOM methods
4. However, the SVGElementInstance
elements generated by SVG use
elements have a rather limited interface which does not include any of those methods, so passing them to a matcher is almost guaranteed to throw.
If wontfix is not an acceptable resolution for the reasons you mentioned, we should do... something... in dispatch
. The most user-friendly method would be similar to cur = cur.correspondingUseElement || cur
within the cur = event.target
loop. !cur.nodeType
(since parentNode
points to the wrong place anyway) is smaller, but would mean losing the event entirely.
Either way, there will be a (small) performance hit in this hot path from the additional property access on cur
.
Changed January 10, 2013 10:45PM UTC by comment:4
owner: | → dmethvin |
---|---|
status: | open → assigned |
Fixing the problem in .dispatch()
does make the most sense. It would be a shame to mess up the matchers with this, since we really shouldn't be passing SVG elements to them in the first place. I'll look at this a bit.
Changed January 11, 2013 02:02AM UTC by comment:5
I think we can just ignore delegated events if the target is SVG and don't even do the bubbling; we'd still allow them on directly bound events as we always have. Since the matcher needs .getAttribute()
we can use that as our marker; for oldIE's sake we have to use typeof
. The only other place where I know we don't have .getAttribute()
is that #12945 bug with Flash in IE9, and if Flash objects start bubbling DOM events out of their guts I don't want to live in this world anymore. That just adds one more condition to the "are there delegated events" check.
if ( delegateCount && typeof event.target.getAttribute !== "undefined" && !(event.button && event.type === "click") ) {
Changed January 11, 2013 03:28AM UTC by comment:6
Why not !event.target.nodeType
? The interface is really restricted. Although now that I think about it, it might be possible to bubble ''up'' into an SVGElementInstance
, which would necessitate dealing with them in the loop.
Changed January 11, 2013 02:28PM UTC by comment:7
Upon further inspection, it appears that SVGElementInstance
s are never the parentNode
s of anything else, which makes our task easier. As for the fix, we can either black-hole events inside of SVG <use>
elements or jump into the real DOM with correspondingUseElement
. The former seems unfairly harsh, but the latter a bit dirty.
In either case, it would be nice to abstract out delegate matching logic into another jQuery.event
method so it can potentially be overridden by people who have strong opinions about the way we've addressed the issues.
Changed January 11, 2013 11:48PM UTC by comment:8
owner: | dmethvin → gibson042 |
---|
Changed January 13, 2013 07:51PM UTC by comment:9
Changed January 14, 2013 12:22AM UTC by comment:10
resolution: | → fixed |
---|---|
status: | assigned → closed |
Fix #13180: don't delegate into SVG <use>
Changeset: f860e0bd2f7dd228a14704d78ed5208cbe870d01
Changed January 14, 2013 01:00AM UTC by comment:11
Fix #13180: don't delegate into SVG <use>
(cherry picked from commits 36457cb6afc12d4a755cf93442a502783a669517..f860e0bd2f7dd228a14704d78ed5208cbe870d01)
Changeset: b75b9ef8d030b0dbf247e826bfbf9585c7dbf98b
Changed January 16, 2013 03:07AM UTC by comment:12
summary: | Live binding click event on image within SVG not firing. → Delegated event not firing for click within SVG use element |
---|
From my tests, this issue occurs in Chrome and IE9, but not FF. Windows Server 2008.
Regarding the guidelines to use jsfiddle; I attempted to use jsfiddle instead of jsbin, but I could not get the SVG to render in jsfiddle.