Skip to main content

Bug Tracker

Side navigation

#6696 closed bug (invalid)

Opened June 21, 2010 02:40AM UTC

Closed November 03, 2010 02:39AM UTC

Last modified June 29, 2012 11:04AM UTC

Delegate does not work with child selector "> *"

Reported by: Phrogz Owned by:
Priority: undecided Milestone: 1.4.3
Component: event Version: 1.4.2
Keywords: delegate child selector live Cc:
Blocked by: Blocking:
Description

A) $("#foo").delegate( "> *", "click", ... ) Doesn't work

B) $("#foo > *").live( "click", ... ) Works

Where "work" is defined as invoking the callback when clicking on a child element of #x. See http://phrogz.net/tmp/delegate_child.html for sample code. (Click on a row of the table or one of the paragraphs to see output in the console.)

Attachments (0)
Change History (12)

Changed June 25, 2010 04:37AM UTC by Phrogz comment:1

Better formatting:

#!c
$("#foo").delegate( "> *", "click", ... ); // Does not work
$("#foo > *").live( "click", ... );        // Works

Changed November 03, 2010 02:39AM UTC by SlexAxton comment:2

_comment0: This is because technically, '> *' is an invalid css selector. You can't start a selector with a child selector. \ \ The correct way to write this as a delegate method would be: \ \ \ {{{ \ // The element you select is where the event is bound \ // The first argument just needs to be a (valid)matching selector for the \ // type of element you wish to fire the event on \ $('#foo').delegate('#foo > *', 'click', ...); \ }}} \ \ I understand there are some catches to make similar things work (via sizzle), but you should try to use valid selectors whenever possible for speed and compatibility purposes. \ \ Thanks for the report!1288752018895086
priority: → undecided
resolution: → invalid
status: newclosed

This is because technically, '> *' is an invalid css selector. You can't start a selector with a child selector.

The correct way to write this as a delegate method would be:

// The element you select is where the event is bound
// The first argument just needs to be a (valid)matching selector for the 
//    type of element you wish to fire the event on
$('#foo').delegate('#foo > *', 'click', ...);

I understand there are some catches to make similar things work (via sizzle), but you should try to use valid selectors whenever possible for speed and compatibility purposes.

Thanks for the report!

Changed November 03, 2010 03:27AM UTC by jitter comment:3

Why would this be an invalid selector just because it starts with a child selector?

There are enough other "functions/operations" in jQuery (e.g. find) where this works as expected. I here also would have assumed that the elements in the jQuery object are used implicitly as context and thus a child selector should surely be legal.

e.g. with find vs delegate. Why would the "same" syntax work for find but not for delegate.

test case shows similar usage.

Changed September 29, 2011 04:52PM UTC by rpflorence@gmail.com comment:4

This is valid. The delegating element is the context of the selector.

Additionally, your element may not have an ID, and may have been dynamically created so your work-around doesn't work.

And finally, consistency. If it works that way in some instances, it should in others.

Changed September 29, 2011 05:56PM UTC by SlexAxton comment:5

_comment0: It's an invalid selector because it does not fall into the spec of the selectors. \ \ It's difficult to implement because `matchesSelector` would be useless (since as ryan pointed out it may not even have an id). \ \ We would then need to create some special parsing cases for this where _if_ it started with a set of special characters to bypass the selector engine, and then remove selectors, and then only run matches selector on the direct children of the root element. This would all be slow and gross. Possible, sure, but an instant anti-pattern for performance in my opinion. \ \ I get why this is useful, but I don't know how to make it worthwhile from a code perspective. I would much prefer a fully qualified selector. \ \ As to Ryan's "dynamically created element with an ID" - nothing is stopping you from giving it one. Then this could be fast by using the fully qualified selector and the native matchesSelector stuffs. \ \ Happy to open back up if people still disagree. I am not the end of the line for the argument.1317319003221256

It's an invalid selector because it does not fall into the specification of valid css selectors.

It's difficult to implement because matchesSelector would be useless (since as ryan pointed out it may not even have an id).

We would then need to create some special parsing cases for this where _if_ it started with a set of special characters to bypass the selector engine, and then remove selectors, and then only run matches selector on the direct children of the root element. This would all be slow and gross. Possible, sure, but an instant anti-pattern for performance in my opinion.

I get why this is useful, but I don't know how to make it worthwhile from a code perspective. I would much prefer a fully qualified selector.

As to Ryan's "dynamically created element with an ID" - nothing is stopping you from giving it one. Then this could be fast by using the fully qualified selector and the native matchesSelector stuffs.

Happy to open back up if people still disagree. I am not the end of the line for the argument.

Changed November 08, 2011 04:47PM UTC by ChrisNielsen comment:6

I'd like to express support for the eventual resolution of this issue.

As previous commenters have stated, there are several other methods in which an initial child selector works as expected. If it works this way in some places but not in all places, there is a needless burden on developers who must now remember an arbitrary rule (use it HERE but not THERE). This seems counterproductive to the spirit of jQuery.

Also, I note that the provided workaround is not intuitive. When using delegate, the selector is already executed within the parent context. The provided workaround only works because the initial ID selector breaks this convention--in my opinion, another bug! For example:

// This should delegate doSomething for all clicks on span tags that
// are descendants from #foo.
$('#foo').delegate('span', 'click', doSomething);

// But if you read this USING THE SAME CONVENTION, it sounds like it is
// trying to delegate doSomething for all clicks on all span tags that are
// direct children of any element with ID of "foo" that is also a descendant
// of #foo:
$('#foo').delegate('#foo > span', 'click', doSomething);

In other words, the workaround reads as though it is expecting this:

<div id="foo">
  <p id="foo">
    <span>...</span>
  </p>
</div>

Of course, this would be an invalid document. I would therefore argue that the workaround is actually an invalid (or at least nonsensical) CSS selector.

Let's take a look at the documentation for the ID selector:

If more than one element has been assigned the same ID, queries that use that ID will only select the first matched element in the DOM. This behavior should not be relied on, however; a document with more than one element using the same ID is invalid.

Ergo, the documentation suggests that we should not be relying on this workaround.

I therefore ask that this bug be reconsidered.

Changed November 08, 2011 05:10PM UTC by dmethvin comment:7

$('#foo').delegate('#foo > span', 'click', doSomething);

Yep, that's the right way to do it. That may look funny because you have #foo in two places, but consider the case where the elements are selected through other means like a class or traversal methods. And the selector you are matching is absolutely correct. You want to match spans that are children of #foo.

So, on the pro side, it saves a few characters in the selector string. On the con side, it's not a standard W3C selector, it's slower, it's more code in core for an edge case, and it has simple alternatives available like attaching a class to the children or just using the current selector.

Changed November 08, 2011 06:13PM UTC by ChrisNielsen comment:8

Sorry, after re-reading my post, I have detected an error in my reasoning:

The workaround is:

$('#foo').delegate('#foo > span', 'click', doSomething);

The signature here is:

$(parent).delegate(selector, eventType, handler);

I failed to realize that selector is always understood to be in the global context. Sorry for my misunderstanding: because handlers attached with delegate can only fire for events which both match selector AND are on the descendant-or-self axis of parent, I incorrectly assumed that selector must function in the context of parent. Indeed, I am surprised that this is not the case; would this not be faster?

Although I am now enlightened, I still believe it would be worthwhile to address this. Much like find, delegate can ONLY operate under a clearly defined context. I think there is a reasonable expectation that it should use its context in doing so.

I also note that the current definition breaks positional selectors, such as "span:first", which may target very different elements if considered in the global context versus a local context.

Changed November 08, 2011 06:30PM UTC by dmethvin comment:9

Hierarchical selectors are best avoided for performance reasons. About two-thirds of all selectors I've seen in the wild are simple non-hierarchical selectors of the form tag#id.class. Adding a class to the children makes this problem go away and is fast.

Don't use positional selectors for delegation, they are not W3C standard and have confusing meaning in a matchesSelector situation.

If you have a patch that can address the issue in a back-compat way we can certainly look at it, with the understanding that we are not likely to use any solution that takes a lot of bytes to implement or makes the common cases slower. It's not on our radar.

Changed February 09, 2012 12:04AM UTC by ryan.wheale@gmail.com comment:10

I would like to make a case for this being re-opened. Take my current situation where I have several un-ordered lists on a single page. Each list has a class of "accordion" and is marked up using your "standard" UL nav markup:

<ul class="accordion">
 <li>
  <a>Heading 1</a>
  <ul>...</ul>
 </li>
 <li>
  <a>Heading 1</a>
  <ul>...</ul>
 </li>
</ul>

And dumbed-down code to allow top level links to serve as accordion headings:

// This does not work
$('.accordion').delegate('> li > a', 'click', function() {
  $(this).next().toggle();
});

// This does work, but wastes resources
$('.accordion').find('> li > a').bind('click', function() {
  $(this).next().toggle();
});

I understand the reasoning behind the proposed workaround... but the argument that this is 'not a standard W3C selector' doesn't sit well when 1) the find() method works, and 2) jQuery is chock full of non-standard selectors. I have been doing jQuery development for years, and I updated an old plugin to use delegate only to find that my selectors didn't work as anticipated.

My proposed solution would be something like this... which should allow matchesSelector to always work.

// at the top of the "on" method (which now replaces delegate)
on: function( types, selector, data, fn, /*INTERNAL*/ one ) {
  [...]
  
  // Shortly before the "return" statement
  // Prepend the selector of "this" to the passed in selector
  if(selector !== undefined) {
    selector = this.selector + ' ' + selector;
  }
  return [...]
}

Changed February 09, 2012 01:09AM UTC by dmethvin comment:11

We're making a strong effort going forward not only to avoid introducing new selector extensions, but to deprecate many existing ones. So the fact they exist today isn't an argument to use them more.

My proposed solution

Actually, this.selector is another thing that isn't reliable either, since it doesn't work correctly on many traversal methods e.g. $("a, button").find("span, b"). So that won't fly, and don't depend on .selector being correct.

I'm not stoked about adding a bunch of code to do this since it's a corner case and it can be accomplished with decent performance by manually doing event delegation. Directly bind an event to your container and then add something like this to the top of the handler:

if ( $(this).children("li").children("a").index(event.target) < 0 )
   return;

http://jsfiddle.net/uPVpH/1/

Changed June 29, 2012 11:04AM UTC by syndicatedshannon comment:12

related: #11993