Skip to main content

Bug Tracker

Side navigation

#11698 closed bug (wontfix)

Opened May 04, 2012 02:24PM UTC

Closed June 02, 2012 06:34PM UTC

Last modified July 17, 2013 10:06AM UTC

$/jQuery doesn't set id when creating element with props

Reported by: tj@crowdersoftware.com Owned by:
Priority: low Milestone: 1.8
Component: core Version: 1.7.2
Keywords: Cc:
Blocked by: Blocking:
Description

When creating elements via jQuery(html, props), the id key doesn't set the ID of the element. This is ''very'' surprising.

Test case | source

jQuery(function($) {

  $("<div>Click me</div>", {
    id: "foo"
  }).appendTo(document.body).click(function() {
    $("<p>").text("My id is " + this.id).appendTo(document.body);
  });
       
});

The result is My id is, rather than My id is foo as expected.

Attachments (0)
Change History (17)

Changed May 04, 2012 02:34PM UTC by tj@crowdersoftware.com comment:1

Ah, it's not just id, it's the properties in general, and the reason is that the string doesn't just have an element tag name on its own, but more general HTML text: http://jsbin.com/exikis | http://jsbin.com/exikis/edit

This is not clear from the documentation, and seems wrong. If just a single element results (or arguably even just a single top-level element), it seems like the properties should be applied to it. That or the documentation should call it out.

Changed May 04, 2012 02:59PM UTC by gibson042 comment:2

This is documented, but I can see a case for always setting attributes on jQuery( html, props )—with a warning against specifying multiple elements in the html.

Changed May 04, 2012 03:04PM UTC by tj@crowdersoftware.com comment:3

@gibson: I don't see anything in the documentation saying that the props will be ignored if the HTML string is more than just a self-contained element tag, what am I reading past? (It's been known to happen.) I see things talking about how the elements are created, but that doesn't mean that the props will (or won't) be ignored.

But yes, it would be great to have the props apply to the top-level returned element(s).

Changed May 04, 2012 03:31PM UTC by dmethvin comment:4

_comment0: > Furthermore, any event type can be passed in, and the following jQuery methods can be called: val, css, html, text, data, width, height, or offset. The name "class" must be quoted in the map since it is a JavaScript reserved word, and "className" cannot be used since it is not the correct attribute name. \ \ I'd just prefer to deprecate the `$(html, props)` form, it's difficult to explain and requires special-case code inside jQuery.1336145572437830
As of jQuery 1.4, the second argument to jQuery() can accept a map consisting of a superset of the properties that can be passed to the .attr() method. Furthermore, any event type can be passed in, and the following jQuery methods can be called: val, css, html, text, data, width, height, or offset. The name "class" must be quoted in the map since it is a JavaScript reserved word, and "className" cannot be used since it is not the correct attribute name.

[Edited, got the incomplete quote] I'd just prefer to deprecate the $(html, props) form, it's difficult to explain and requires special-case code inside jQuery.

Changed May 04, 2012 03:32PM UTC by scottgonzalez comment:5

Does that mean .attr( props, true ) would go away too? This is really useful and we use it in jQuery UI.

Changed May 04, 2012 03:34PM UTC by dmethvin comment:6

@scott.gonzalez no, that would be the preferred way to do it.

Changed May 04, 2012 03:55PM UTC by T.J. Crowder <tj@crowdersoftware.com> comment:7

@dmethvin: I don't understand what you're saying with that quote...? It doesn't say anything about props being ignored in one case but not in another.

FWIW, anything that reduces the outrageous overloading of $(), I'm in favor of. :-) I very rarely use this form and would be happy to use attr instead. But I ''see'' it used a lot, I suspect a fair bit of backlash if it were deprecated.

Changed May 04, 2012 04:03PM UTC by jaubourg comment:8

Replying to [comment:4 dmethvin]:

> As of jQuery 1.4, the second argument to jQuery() can accept a map consisting of a superset of the properties that can be passed to the .attr() method. Furthermore, any event type can be passed in, and the following jQuery methods can be called: val, css, html, text, data, width, height, or offset. The name "class" must be quoted in the map since it is a JavaScript reserved word, and "className" cannot be used since it is not the correct attribute name. [Edited, got the incomplete quote] I'd just prefer to deprecate the $(html, props) form, it's difficult to explain and requires special-case code inside jQuery.

I'm not with you on this one, Dave. I think $( html, props ) is probably the neatest way to create elements in jQuery. It makes for very readable code. $.attr( object, true ) not only adds noise but is also quite esoteric in comparison.

Changed May 04, 2012 04:19PM UTC by gibson042 comment:9

Replying to [comment:3 tj@…]:

@gibson: I don't see anything in the documentation saying that the props will be ignored if the HTML string is more than just a self-contained element tag, what am I reading past? (It's been known to happen.) I see things talking about how the elements are created, but that doesn't mean that the props will (or won't) be ignored.

Emphasis mine:

jQuery( html, props )

>

html A string defining ''a single, standalone, HTML element'' (e.g. <div/> or <div></div>).

>

props A map of attributes, events, and methods to call on the newly-created element.

Changed May 04, 2012 04:22PM UTC by rwaldron comment:10

From http://api.jquery.com/jQuery/#jQuery2

A string defining a single, standalone, HTML element (e.g. <div/> or <div></div>).

But I'm not sure this precludes <div>foo</div>, if so the docs need to be clearer.

I got blitzed by @ gibson042

Changed May 04, 2012 04:24PM UTC by rwaldron comment:11

Turns out there is only one supporting test for this signature. I also think that T.J.'s assumption makes sense. It's still one element.

Changed May 04, 2012 04:24PM UTC by rwaldron comment:12

component: unfiledcore
milestone: None1.8
owner: → rwaldron
priority: undecidedlow
status: newassigned

Changed May 04, 2012 04:32PM UTC by dmethvin comment:13

@tj, sorry I was just confirming that it seemed like someone could be lulled into thinking it should work.

@jaubourg, you're right I misunderstood scott's comment. My brain saw the props and thought .prop. The pass arg to jQuery.attr is not even documented!

@rwaldron, yeah the confusion is because the case where it applies props is only when we recognize the simple single-element string, not something like $('<div id="abc">hello</div>", { "class": "def" });

The docs there are wrong in several ways, which is why I prefer deprecation as the easiest way out:

It's not true that "any event type" is accepted, it's only a short list that you would find only in the code. Just as with the single-elementness, this list can be documented but it makes the feature even more confusing and esoteric.

If it's not one of the recognized event names or methods in jQuery.attrFn we assume it's an attribute. However, since 1.6 we've been trying to draw the line between proper use of attributes and properties. This code always goes through .attr() and therefore would need updating to use .prop() in certain known cases once the "yer doin it rong but we'll save you" attribute-setter shims are removed. Or, we can never remove the shims.

This isn't even about brevity, we have a perfectly good chaining syntax that is not ambiguous, just about as brief as $(html, props) and doesn't require memorizing a long paragraph of special cases.

As far as deprecation goes, again it doesn't mean we *must* take it out. It means we know it has drawbacks, we can't easily fix them, and don't advise you use it in new code.

Changed May 04, 2012 04:35PM UTC by gibson042 comment:14

@rwaldron, the relevant fork uses document.createElement instead of jQuery.buildFragment when rsingleTag = /^<(\\w+)\\s*\\/?>(?:<\\/\\1>)?$/ matches. It doesn't make sense to allow only text content and the regex is useless if we allow complex content, so we should either leave this alone or set attributes whenever the second argument is a plain object.

Changed May 04, 2012 04:37PM UTC by rwaldron comment:15

_comment0: I just poked around a bit and my first thoughts about the conditional matching that would likely be required to make this work is not worth it.1336149489814850
owner: rwaldron
status: assignedopen

I just poked around a bit and my first thoughts about the conditional matching that would likely be required to make this work is not worth it.

@gibson042 blitzed me... again!

Changed June 02, 2012 06:34PM UTC by dmethvin comment:16

resolution: → wontfix
status: openclosed

I still feel like $(html, props) is a minefield. Even the abbreviation is deceptive since props are either event handlers or *attributes*; properties are only handled correctly due to the grace of special case attrHooks we want to remove. So let's not pick at this, we'll make it bleed.

Changed July 17, 2013 10:06AM UTC by bobince@gmail.com comment:17

This also affects elements with attributes, ie:

$('<div class="a">', {id: 'b'})

fails.

If this behaviour isn't going to be fixed, let's deprecate the feature entirely - at the moment it is a surprising trap.