Skip to main content

Bug Tracker

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 dankellett 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

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.

Changed January 08, 2013 07:52PM UTC by dmethvin comment:2

cc: → gibson042, timmywil
component: unfiledevent
milestone: None1.9
priority: undecidedlow
status: newopen

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 gibson042 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 dmethvin comment:4

owner: → dmethvin
status: openassigned

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 dmethvin 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 gibson042 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 gibson042 comment:7

Upon further inspection, it appears that SVGElementInstances are never the parentNodes 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 gibson042 comment:8

owner: dmethvingibson042

Changed January 13, 2013 07:51PM UTC by gibson042 comment:9

Changed January 14, 2013 12:22AM UTC by Richard Gibson comment:10

resolution: → fixed
status: assignedclosed

Fix #13180: don't delegate into SVG <use>

Changeset: f860e0bd2f7dd228a14704d78ed5208cbe870d01

Changed January 14, 2013 01:00AM UTC by Richard Gibson 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 gibson042 comment:12

summary: Live binding click event on image within SVG not firing.Delegated event not firing for click within SVG use element