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.
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 comment:1
Changed May 04, 2012 02:59PM UTC by 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 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 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 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 comment:6
@scott.gonzalez no, that would be the preferred way to do it.
Changed May 04, 2012 03:55PM UTC by 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 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 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 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 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 comment:12
component: | unfiled → core |
---|---|
milestone: | None → 1.8 |
owner: | → rwaldron |
priority: | undecided → low |
status: | new → assigned |
Changed May 04, 2012 04:32PM UTC by 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 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 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: | assigned → open |
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 comment:16
resolution: | → wontfix |
---|---|
status: | open → closed |
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 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.
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/editThis 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.