I ran into our coding conventions for creating elements in JS: https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Creating... var $hello = $('<div>').text( 'Hello' ); // Not '<div/>' // Not '<div></div>'
This looks like some really bad advice.
This dates back to an issue I ran into with jQuery 3 years ago: https://forum.jquery.com/topic/ie-issue-with-append#14737000000469433 https://forum.jquery.com/topic/ie-issue-with-append#14737000000469445 Basically $( '<span class="foo">' ) will break completely in IE7/IE8.
jQuery supports /> on all elements (eg: $( '<span class="foo" />' )) intentionally. It does internal replacements to support it as a syntax for specifying elements. But besides that special case jQuery wants anything passed to it to be valid markup. It just leaves the parsing of it up to the browser and all the quirks the browser may have. jQuery does special case attribute-less $( '<div />' ) but this is a performance enhancement. The fact that $( '<div>' ) does not break in IE7/IE8 is an unintentional side effect of jQuery's lazy support of special cases like $( '<img>' ) where the tag is self closing and the browser will not require a /. This is the ONLY case where jQuery intentionally supports leaving out a closing tag or a self-closing /.
When devs consider `$( '<div>' )` ok they typically believe that `$( '<div class="foo">' )` should be ok to. It looks like a special cased way to define an element, attributes et. all. However the former is a performance enhancement side effect, and the later while it will look like it works in Chrome and Firefox actually relies entirely on quirky error correction behavior which differs between engines and breaks in IE7/IE8 engine.
The jQuery documentation also does not state that `$( '<div>' )` for non-selfclosing elements is supported: http://api.jquery.com/jQuery/#jQuery2
Hence, I think we should change our coding conventions to always use `$( '<div />' )`.
((And for anyone that suggests that developers should know they should add a / or </div> to <div> when they add any attributes to it. I bring up the fact that our code style requires {} blocks and does not allow implicit `if ( foo ) foo();`. This is the same thing.))
Hence, I think we should change our coding conventions to always use `$( '<div />' )`.
+1 for valid XHTML. Considering that bytes are cheap and validity is good, this seems like a good idea.
I also tried to get an answer about the better between $( '<div class="a-class" />' ) and $( '<div />' ).addClass( 'a-class' ), but apparently there's little difference. At least when creating dynamic interfaces, I'd like to have some guidance and consistency if anyone is interested in chatting about it.
My preference is the latter, because it avoids extensive HTML inside of JavaScript. But I'd be interested to hear other thoughts.
$( '<div>' ) is the way to go.
- Trevor
On Mon, Aug 27, 2012 at 4:37 PM, Mark Holmquist mtraceur@member.fsf.orgwrote:
Hence, I think we should change our coding conventions to always use `$(
'<div />' )`.
+1 for valid XHTML. Considering that bytes are cheap and validity is good, this seems like a good idea.
I also tried to get an answer about the better between $( '<div class="a-class" />' ) and $( '<div />' ).addClass( 'a-class' ), but apparently there's little difference. At least when creating dynamic interfaces, I'd like to have some guidance and consistency if anyone is interested in chatting about it.
My preference is the latter, because it avoids extensive HTML inside of JavaScript. But I'd be interested to hear other thoughts.
-- Mark Holmquist Contractor, Wikimedia Foundation mtraceur@member.fsf.org http://marktraceur.info
______________________________**_________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/**mailman/listinfo/wikitech-lhttps://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Monday, August 27, 2012 at 4:40 PM, Trevor Parscal wrote:
$( '<div>' ) is the way to go.
Yeah. Mark Pilgrim's overview of the sordid history of XHTML is useful background: http://diveintohtml5.info/past.html
-- Ori Livneh ori@wikimedia.org
On Mon, 27 Aug 2012 16:57:52 -0700, Ori Livneh ori@wikimedia.org wrote:
On Monday, August 27, 2012 at 4:40 PM, Trevor Parscal wrote:
$( '<div>' ) is the way to go.
Yeah. Mark Pilgrim's overview of the sordid history of XHTML is useful background: http://diveintohtml5.info/past.html
-- Ori Livneh ori@wikimedia.org
Can we not just jump into making this a HTML5 vs. XHTML topic?
Because if you want to make this HTML5 vs. XHTML '<div>' is NOT valid HTML5.
If you don't like the XHTML-ish shortcut that jQuery provides, then our coding conventions should be to use `$( '<div></div>' );`.
Either way $( '<div>' ) is NOT something officially supported by jQuery and makes it easy for developers to accidentally write broken code.
On Monday, August 27, 2012 at 5:50 PM, Daniel Friesen wrote:
If you don't like the XHTML-ish shortcut that jQuery provides, then our coding conventions should be to use `$( '<div></div>' );`.
Either way $( '<div>' ) is NOT something officially supported by jQuery and makes it easy for developers to accidentally write broken code.
By Jove, you're right. The jQuery docs confirm. Retracted!
-- Ori Livneh ori@wikimedia.org
On Mon, Aug 27, 2012 at 4:37 PM, Mark Holmquist mtraceur@member.fsf.org wrote:
I also tried to get an answer about the better between $( '<div class="a-class" />' ) and $( '<div />' ).addClass( 'a-class' ), but apparently there's little difference. At least when creating dynamic interfaces, I'd like to have some guidance and consistency if anyone is interested in chatting about it.
I'm going to try and put some guidelines for secure javascript code together soon, but it's a much better habit to use .addClass( 'a-class' ) and the other functions to add attributes.
On Mon, Aug 27, 2012 at 9:39 PM, Chris Steipp csteipp@wikimedia.org wrote:
On Mon, Aug 27, 2012 at 4:37 PM, Mark Holmquist mtraceur@member.fsf.org wrote:
I also tried to get an answer about the better between $( '<div class="a-class" />' ) and $( '<div />' ).addClass( 'a-class' ), but apparently there's little difference. At least when creating dynamic interfaces, I'd like to have some guidance and consistency if anyone is interested in chatting about it.
I'm going to try and put some guidelines for secure javascript code together soon, but it's a much better habit to use .addClass( 'a-class' ) and the other functions to add attributes.
To clarify this point, in that *specific* example, there's no appreciable difference from a security perspective.
The problem comes when you move out of the land of constants, and start concatenating variables. Any time you find yourself doing something like this: $( '<div class="' + fooclass + '" />' );
...you're way better off doing this: $( '<div />' ).addClass( fooclass );
Not only is it clearer, but it's more secure. Why? If the provenance of that variable is at all unclear, or if you know it comes from user input, I believe you're basically using the DOM to make sure that the string is treated as a class name rather than arbitrary HTML (Arbitrary HTML leads to XSS. XSS leads to anger. Anger leads to hate....). I don't know jQuery well enough to know for sure if using addClass is sufficient for arbitrary strings or if it's best to *also* escape fooclass, but it's at least more likely to be safe than concatenating to arbitrary HTML.
If you're dealing just with a constant string, maybe it's ok to use either. However, sooner or later, someone is going to want to make a small tweak that involves changing part of the constant to a variable, and there's a good chance that someone is going to be relatively inexperienced and will simply rely on concatenation rather than changing the code to use addClass.
Even if addClass doesn't inherently handle the escaping for you, a lot of basic DOM methods do, which is what makes them appealing from a security perspective. The more specific you can be, the better.
Rob
Rob is correct that using addClass is the preferable way to go for classes, and attr is the preferable way to go for other attributes. They are both are safe since they use setAttribute internally which escapes the values.
- Trevor
On Tue, Aug 28, 2012 at 10:29 AM, Rob Lanphier robla@wikimedia.org wrote:
On Mon, Aug 27, 2012 at 9:39 PM, Chris Steipp csteipp@wikimedia.org wrote:
On Mon, Aug 27, 2012 at 4:37 PM, Mark Holmquist mtraceur@member.fsf.org
wrote:
I also tried to get an answer about the better between $( '<div class="a-class" />' ) and $( '<div />' ).addClass( 'a-class' ), but apparently there's little difference. At least when creating dynamic interfaces, I'd like to have some guidance and consistency if anyone is interested in chatting about it.
I'm going to try and put some guidelines for secure javascript code together soon, but it's a much better habit to use .addClass( 'a-class' ) and the other functions to add attributes.
To clarify this point, in that *specific* example, there's no appreciable difference from a security perspective.
The problem comes when you move out of the land of constants, and start concatenating variables. Any time you find yourself doing something like this: $( '<div class="' + fooclass + '" />' );
...you're way better off doing this: $( '<div />' ).addClass( fooclass );
Not only is it clearer, but it's more secure. Why? If the provenance of that variable is at all unclear, or if you know it comes from user input, I believe you're basically using the DOM to make sure that the string is treated as a class name rather than arbitrary HTML (Arbitrary HTML leads to XSS. XSS leads to anger. Anger leads to hate....). I don't know jQuery well enough to know for sure if using addClass is sufficient for arbitrary strings or if it's best to *also* escape fooclass, but it's at least more likely to be safe than concatenating to arbitrary HTML.
If you're dealing just with a constant string, maybe it's ok to use either. However, sooner or later, someone is going to want to make a small tweak that involves changing part of the constant to a variable, and there's a good chance that someone is going to be relatively inexperienced and will simply rely on concatenation rather than changing the code to use addClass.
Even if addClass doesn't inherently handle the escaping for you, a lot of basic DOM methods do, which is what makes them appealing from a security perspective. The more specific you can be, the better.
Rob
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On 12-08-28 10:40 AM, Trevor Parscal wrote:
Rob is correct that using addClass is the preferable way to go for classes, and attr is the preferable way to go for other attributes. They are both are safe since they use setAttribute internally which escapes the values.
In creating elements, maybe, but after creation, $.prop() is the preferred way to go because the DOM properties are more reliably synced with the actual state of the UI--apparently jQuery doesn't always properly sync the HTML attributes to the browser state. I'm sure Timo can explain more fully (and maybe more accurately).
jQuery internally maps '<tagName>' to document.createElement( 'tagName' ). This is a feature, and is used throughout jQuery internally. It's not very well documented as such, but Timo is adding it to the documentation as to resolve the confusion around this. $( '<div>' ) is a shortcut added to jQuery for our convenience, and I think it's reasonable to use it.
On Tue, Aug 28, 2012 at 10:44 AM, Mark Holmquist mtraceur@member.fsf.orgwrote:
In creating elements, maybe, but after creation, $.prop() is the preferred way to go because the DOM properties are more reliably synced with the actual state of the UI--apparently jQuery doesn't always properly sync the HTML attributes to the browser state. I'm sure Timo can explain more fully (and maybe more accurately).
We had this discussion yesterday, and addClass is more direct than prop( 'className' ) in every way and unless you mean to actually replace all existing classes, addClass is preferred. prop is there for a reason and it's also safe to use as escaping goes, but obviously not all attributes are actually properties, so it's not like we should stop using attr.
- Trevor
On 8/28/12 10:52 AM, Trevor Parscal wrote:
jQuery internally maps '<tagName>' to document.createElement( 'tagName' ). This is a feature, and is used throughout jQuery internally. It's not very well documented as such, but Timo is adding it to the documentation as to resolve the confusion around this. $( '<div>' ) is a shortcut added to jQuery for our convenience, and I think it's reasonable to use it.
In that case, perhaps we should just say that all of the options are fine: $( '<div>' ) $( '<div/>' ) $( '<div></div>' ) but emphasize not to use attributes in the tag creation.
Ryan Kaldari
On Tue, Aug 28, 2012 at 11:00 AM, Ryan Kaldari rkaldari@wikimedia.orgwrote:
In that case, perhaps we should just say that all of the options are fine: $( '<div>' ) $( '<div/>' ) $( '<div></div>' ) but emphasize not to use attributes in the tag creation.
Unless you are creating an input or a button. Maybe, we should encourage people to read the jQuery documentation instead of trying to re-document every feature ourselves.
- Trevor
In that case, perhaps we should just say that all of the options are fine: $( '<div>' ) $( '<div/>' ) $( '<div></div>' ) but emphasize not to use attributes in the tag creation.
+1 All three will return the same, and this is not HTML, it's JavaScript. It really is just a matter of personal flavor in coding style, nothing else.
By the way, you can also use
$( '<div/>', { 'class': 'foo', 'title': 'myTitle', ... } );
for adding attributes right away. Should be faster, more readable and less error-prone than using tags directly in the div's definition string.
Cheers, Daniel W
On Thu, Aug 30, 2012 at 3:24 AM, Daniel Werner daniel.werner@wikimedia.dewrote:
By the way, you can also use
$( '<div/>', { 'class': 'foo', 'title': 'myTitle', ... } );
Just be aware this also allows you to use things like 'html' and 'text' which are not attributes at all, but call .html() or .text() internally. There are also property aliases here.
In short - be aware of what the code does by reading the manual.
- Trevor
Timo where are you ?
+1 .addClass as it directly manipulates the .className property.
On Aug 28, 2012, at 10:52 AM, Trevor Parscal wrote:
jQuery internally maps '<tagName>' to document.createElement( 'tagName' ). This is a feature, and is used throughout jQuery internally. It's not very well documented as such, but Timo is adding it to the documentation as to resolve the confusion around this. $( '<div>' ) is a shortcut added to jQuery for our convenience, and I think it's reasonable to use it.
On Tue, Aug 28, 2012 at 10:44 AM, Mark Holmquist mtraceur@member.fsf.orgwrote:
In creating elements, maybe, but after creation, $.prop() is the preferred way to go because the DOM properties are more reliably synced with the actual state of the UI--apparently jQuery doesn't always properly sync the HTML attributes to the browser state. I'm sure Timo can explain more fully (and maybe more accurately).
We had this discussion yesterday, and addClass is more direct than prop( 'className' ) in every way and unless you mean to actually replace all existing classes, addClass is preferred. prop is there for a reason and it's also safe to use as escaping goes, but obviously not all attributes are actually properties, so it's not like we should stop using attr.
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Unfortunately, our resident Javascript master is eating dinner :)
Ryan Kaldari
On 8/28/12 11:00 AM, Rob Moen wrote:
Timo where are you ?
+1 .addClass as it directly manipulates the .className property.
On Aug 28, 2012, at 10:52 AM, Trevor Parscal wrote:
jQuery internally maps '<tagName>' to document.createElement( 'tagName' ). This is a feature, and is used throughout jQuery internally. It's not very well documented as such, but Timo is adding it to the documentation as to resolve the confusion around this. $( '<div>' ) is a shortcut added to jQuery for our convenience, and I think it's reasonable to use it.
On Tue, Aug 28, 2012 at 10:44 AM, Mark Holmquist mtraceur@member.fsf.orgwrote:
In creating elements, maybe, but after creation, $.prop() is the preferred way to go because the DOM properties are more reliably synced with the actual state of the UI--apparently jQuery doesn't always properly sync the HTML attributes to the browser state. I'm sure Timo can explain more fully (and maybe more accurately).
We had this discussion yesterday, and addClass is more direct than prop( 'className' ) in every way and unless you mean to actually replace all existing classes, addClass is preferred. prop is there for a reason and it's also safe to use as escaping goes, but obviously not all attributes are actually properties, so it's not like we should stop using attr.
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Aug 28, 2012, at 10:52 AM, Trevor Parscal wrote:
jQuery internally maps '<tagName>' to document.createElement( 'tagName' ). This is a feature, and is used throughout jQuery internally. It's not very well documented as such, but Timo is adding it to the documentation as to resolve the confusion around this. $( '<div>' ) is a shortcut added to jQuery for our convenience, and I think it's reasonable to use it.
+ 1 I've always used '<div />'. Recently though, I learned that you no longer need to do that in jQuery. And since have been using '<div>'.
On Tue, Aug 28, 2012 at 10:44 AM, Mark Holmquist mtraceur@member.fsf.orgwrote:
In creating elements, maybe, but after creation, $.prop() is the preferred way to go because the DOM properties are more reliably synced with the actual state of the UI--apparently jQuery doesn't always properly sync the HTML attributes to the browser state. I'm sure Timo can explain more fully (and maybe more accurately).
We had this discussion yesterday, and addClass is more direct than prop( 'className' ) in every way and unless you mean to actually replace all existing classes, addClass is preferred. prop is there for a reason and it's also safe to use as escaping goes, but obviously not all attributes are actually properties, so it's not like we should stop using attr.
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Tue, 28 Aug 2012 11:12:22 -0700, Rob Moen rmoen@wikimedia.org wrote:
On Aug 28, 2012, at 10:52 AM, Trevor Parscal wrote:
jQuery internally maps '<tagName>' to document.createElement( 'tagName' ). This is a feature, and is used throughout jQuery internally. It's not very well documented as such, but Timo is adding it to the documentation as to resolve the confusion around this. $( '<div>' ) is a shortcut added to jQuery for our convenience, and I think it's reasonable to use it.
- 1
I've always used '<div />'. Recently though, I learned that you no longer need to do that in jQuery. And since have been using '<div>'.
That's an unintentional side effect. jQuery does not officially support $( '<div>' ) without a closing </div> or />.
On Tue, Aug 28, 2012 at 10:44 AM, Mark Holmquist mtraceur@member.fsf.orgwrote:
In creating elements, maybe, but after creation, $.prop() is the preferred way to go because the DOM properties are more reliably synced with the actual state of the UI--apparently jQuery doesn't always properly sync the HTML attributes to the browser state. I'm sure Timo can explain more fully (and maybe more accurately).
We had this discussion yesterday, and addClass is more direct than prop( 'className' ) in every way and unless you mean to actually replace all existing classes, addClass is preferred. prop is there for a reason and it's also safe to use as escaping goes, but obviously not all attributes are actually properties, so it's not like we should stop using attr.
- Trevor
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Tue, Aug 28, 2012 at 11:15 AM, Daniel Friesen <daniel@nadir-seen-fire.com
wrote:
That's an unintentional side effect. jQuery does not officially support $( '<div>' ) without a closing </div> or />.
And yet they use it themselves internally? As I mentioned, Timo is a jQuery maintainer and said it is supported and welcome to be used, and is adding the regrettably lacking documentation soon.
- Trevor
Okay, sorry for being away for 30 minutes while I enjoyed dinner.
Someone[1] pointed me to this thread and suggested I chime in, so here I go.
On Aug 28, 2012, at 2:50 AM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
Either way $( '<div>' ) is NOT something officially supported by jQuery [..]
This is simply incorrect. * It has been explicitly supported (whether or not intentionally/officiallt) by jQuery for years as can be seen in the source code. * It has been used by jQuery core and other jQuery project for years (not just sporadically, but pretty much everywhere, consistency).
And I'm happy to announce that as of today, by popular demand, the jQuery team has finally updated[4] the 3-year old documentation to reflect this feature.
Up until now the documentation for the root jQuery() function still reflected the situation as it was 3 years ago. Where the string always had to be fully valid with closing tag, with the exception of <input> and <img/> because the native parsers in the browsers supported it that way (not because jQuery wanted us to).
But for a while now jQuery features element creation through the native createElement() by passing a single <tag> ("with optional closing tag or quick-closing"[2]). As such I've reverted this edit[3].
On Aug 28, 2012, at 9:57 AM, Tim Starling tstarling@wikimedia.org wrote:
Personally, I would use document.getElementById() to do that. It's standard, and it's faster and more secure. More complex selectors derived from user input can be replaced with jQuery.filter() etc. with no loss of performance.
-- Tim Starling
Indeed. Moreover, aside from the performance and security, there's another important factor to take into account. And that is the fact that IDs can contain characters that have special meaning in CSS selectors (such as dots).
We've seen this in before when dealing with a MediaWiki heading (where the ID-version of the heading can (or could) contain dots). So whenever you have what is supposed to be an element ID in a variable, use document.getElementById (even if you don't care about performance or security).
On Aug 28, 2012, at 6:39 AM, Chris Steipp csteipp@wikimedia.org wrote:
On Mon, Aug 27, 2012 at 4:37 PM, Mark Holmquist mtraceur@member.fsf.org wrote:
I also tried to get an answer about the better between $( '<div class="a-class" />' ) and $( '<div />' ).addClass( 'a-class' ), but apparently there's little difference. At least when creating dynamic interfaces, I'd like to have some guidance and consistency if anyone is interested in chatting about it.
I'm going to try and put some guidelines for secure javascript code together soon, but it's a much better habit to use .addClass( 'a-class' ) and the other functions to add attributes.
I'm looking forward to that.
Note that it is perfectly fine and secure to use: $( '<div class="a-class"></div>' );
But when working with variables (whether from user input or not), then methods like addClass should be used instead. Both for security as well as predictability: $( '<div class="' + someVar + '"></div>' ); // Bad
If the variable contains any unexpected characters it can for example cause the jQuery object to be a collection of 2 or 3 elements instead of 1.
On Aug 28, 2012, at 8:00 PM, Ryan Kaldari rkaldari@wikimedia.org wrote:
In that case, perhaps we should just say that all of the options are fine: $( '<div>' ) $( '<div/>' ) $( '<div></div>' )
Indeed[5].
On Aug 28, 2012, at 2:50 AM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
If you don't like the XHTML-ish shortcut that jQuery provides, then our coding conventions should be to use `$( '<div></div>' );`.
I agree we shouldn't use XHTML-ish shortcuts because it looks confusing: $('<ul><li/></ul>');
That works because jQuery converts <tag/> to <tag></tag>. But just because jQuery allows that doesn't mean we should do it. I'd recommend we keep it simple and always use valid HTML syntax when writing HTML snippets for parsing.
Either use the <tag> syntax to create a plain element, or use fully valid XML/HTML syntax (with no shortcuts) for everything else.
-- Timo
[1] Well, actually, almost a dozen someones.
[2] http://api.jquery.com/jQuery/?purge=1#jQuery2
[3] https://www.mediawiki.org/w/index.php?title=Manual%3ACoding_conventions%2FJa...
[4] https://github.com/jquery/api.jquery.com/commit/ea8d2857cd23b2044948a15708a2...
[5] https://www.mediawiki.org/w/index.php?title=Manual%3ACoding_conventions%2FJa...
+1
Thank you for grounding this conversation in reality.
- Trevor
On Tue, Aug 28, 2012 at 12:18 PM, Krinkle krinklemail@gmail.com wrote:
Okay, sorry for being away for 30 minutes while I enjoyed dinner.
Someone[1] pointed me to this thread and suggested I chime in, so here I go.
On Aug 28, 2012, at 2:50 AM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
Either way $( '<div>' ) is NOT something officially supported by jQuery
[..]
This is simply incorrect.
- It has been explicitly supported (whether or not
intentionally/officiallt) by jQuery for years as can be seen in the source code.
- It has been used by jQuery core and other jQuery project for years (not
just sporadically, but pretty much everywhere, consistency).
And I'm happy to announce that as of today, by popular demand, the jQuery team has finally updated[4] the 3-year old documentation to reflect this feature.
Up until now the documentation for the root jQuery() function still reflected the situation as it was 3 years ago. Where the string always had to be fully valid with closing tag, with the exception of <input> and <img/> because the native parsers in the browsers supported it that way (not because jQuery wanted us to).
But for a while now jQuery features element creation through the native createElement() by passing a single <tag> ("with optional closing tag or quick-closing"[2]). As such I've reverted this edit[3].
On Aug 28, 2012, at 9:57 AM, Tim Starling tstarling@wikimedia.org wrote:
Personally, I would use document.getElementById() to do that. It's standard, and it's faster and more secure. More complex selectors derived from user input can be replaced with jQuery.filter() etc. with no loss of performance.
-- Tim Starling
Indeed. Moreover, aside from the performance and security, there's another important factor to take into account. And that is the fact that IDs can contain characters that have special meaning in CSS selectors (such as dots).
We've seen this in before when dealing with a MediaWiki heading (where the ID-version of the heading can (or could) contain dots). So whenever you have what is supposed to be an element ID in a variable, use document.getElementById (even if you don't care about performance or security).
On Aug 28, 2012, at 6:39 AM, Chris Steipp csteipp@wikimedia.org wrote:
On Mon, Aug 27, 2012 at 4:37 PM, Mark Holmquist mtraceur@member.fsf.org
wrote:
I also tried to get an answer about the better between $( '<div class="a-class" />' ) and $( '<div />' ).addClass( 'a-class' ), but apparently there's little difference. At least when creating dynamic interfaces, I'd like to have some guidance and consistency if anyone is interested in chatting about it.
I'm going to try and put some guidelines for secure javascript code together soon, but it's a much better habit to use .addClass( 'a-class' ) and the other functions to add attributes.
I'm looking forward to that.
Note that it is perfectly fine and secure to use: $( '<div class="a-class"></div>' );
But when working with variables (whether from user input or not), then methods like addClass should be used instead. Both for security as well as predictability: $( '<div class="' + someVar + '"></div>' ); // Bad
If the variable contains any unexpected characters it can for example cause the jQuery object to be a collection of 2 or 3 elements instead of 1.
On Aug 28, 2012, at 8:00 PM, Ryan Kaldari rkaldari@wikimedia.org wrote:
In that case, perhaps we should just say that all of the options are
fine:
$( '<div>' ) $( '<div/>' ) $( '<div></div>' )
Indeed[5].
On Aug 28, 2012, at 2:50 AM, Daniel Friesen daniel@nadir-seen-fire.com wrote:
If you don't like the XHTML-ish shortcut that jQuery provides, then our
coding conventions should be to use `$( '<div></div>' );`.
I agree we shouldn't use XHTML-ish shortcuts because it looks confusing: $('<ul><li/></ul>');
That works because jQuery converts <tag/> to <tag></tag>. But just because jQuery allows that doesn't mean we should do it. I'd recommend we keep it simple and always use valid HTML syntax when writing HTML snippets for parsing.
Either use the <tag> syntax to create a plain element, or use fully valid XML/HTML syntax (with no shortcuts) for everything else.
-- Timo
[1] Well, actually, almost a dozen someones.
[2] http://api.jquery.com/jQuery/?purge=1#jQuery2
[3] https://www.mediawiki.org/w/index.php?title=Manual%3ACoding_conventions%2FJa...
[4] https://github.com/jquery/api.jquery.com/commit/ea8d2857cd23b2044948a15708a2...
[5] https://www.mediawiki.org/w/index.php?title=Manual%3ACoding_conventions%2FJa...
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
Indeed. Moreover, aside from the performance and security, there's another important factor to take into account. And that is the fact that IDs can contain characters that have special meaning in CSS selectors (such as dots).
We've seen this in before when dealing with a MediaWiki heading (where the ID-version of the heading can (or could) contain dots). So whenever you have what is supposed to be an element ID in a variable, use document.getElementById (even if you don't care about performance or security).
Modern browsers like Chrome / IE9 are really fast, jQuery isn't just about performance but also about shorter code. Why don't have something like $.id(myId), as a "shortcut" to document.getElementById(). Also perhaps another .get* methods (there were bunch of them). As you are one of jQuery code maintainers, such feature can be easily implemented. Dmitriy
On Tue, Aug 28, 2012 at 4:18 PM, Krinkle krinklemail@gmail.com wrote:
Moreover, aside from the performance and security, there's another important factor to take into account. And that is the fact that IDs can contain characters that have special meaning in CSS selectors (such as dots).
We've seen this in before when dealing with a MediaWiki heading (where the ID-version of the heading can (or could) contain dots). So whenever you have what is supposed to be an element ID in a variable, use document.getElementById (even if you don't care about performance or security).
About that, see: https://bugzilla.wikimedia.org/30471 (MediaWiki generates ids which can't be selected)
Helder
I will rescue two facts listed in this thread, about using jquery and creating tags
[quote][1]
Basically $( '<span class="foo">' ) will break completely in IE7/IE8.
[quote][2]
It's important to note however that IE required that input and button >tags are created with a type (if they are going to have a specific one)
$( '<input type="password">', { 'class', 'example' } );
On 28/08/12 09:26, Daniel Friesen wrote:
jQuery does special case attribute-less $( '<div />' ) but this is a performance enhancement. The fact that $( '<div>' ) does not break in IE7/IE8 is an unintentional side effect of jQuery's lazy support of special cases like $( '<img>' ) where the tag is self closing and the browser will not require a /.
The performance special case supports both <div> and <div/>:
rsingleTag = /^<(\w+)\s*/?>$/, ... ret = rsingleTag.exec( selector ); if ( ret ) { selector = [ doc.createElement( ret[1] ) ];
I think that we should use that special case, and then extend the resulting elements with attr() rather than hitting the innerHTML case. When you specify longer HTML fragments as strings, they tend to get polluted with user input, leading to XSS.
But it's important to establish conventions which lead the developer down a path where they're less likely to run into trouble, even if they do try to take a few shortcuts. So let's add the slash.
(There is one important case where it's good to use innerHTML: when there are thousands of elements to create in a batch.)
-- Tim Starling
On Mon, 27 Aug 2012 19:22:25 -0700, Tim Starling tstarling@wikimedia.org wrote:
On 28/08/12 09:26, Daniel Friesen wrote:
jQuery does special case attribute-less $( '<div />' ) but this is a performance enhancement. The fact that $( '<div>' ) does not break in IE7/IE8 is an unintentional side effect of jQuery's lazy support of special cases like $( '<img>' ) where the tag is self closing and the browser will not require a /.
The performance special case supports both <div> and <div/>:
rsingleTag = /^<(\w+)\s*/?>$/, ... ret = rsingleTag.exec( selector ); if ( ret ) { selector = [ doc.createElement( ret[1] ) ];
The support of <div> is an unintended side effect. Basically jQuery officially supports <div />, <img />, and <img>. Where the non-/> version is only supported for elements that can't have content like img, input, etc... The fact that the createElement special case makes <div> work is a side effect of the /? added to support <img>. It would have been more code to explicitly break the input that they don't officially support.
I think that we should use that special case, and then extend the resulting elements with attr() rather than hitting the innerHTML case. When you specify longer HTML fragments as strings, they tend to get polluted with user input, leading to XSS.
Personally. When it comes to creating a single attribute-less element I'd rather avoid hitting any special magical parsing routines. When you do $( '<div />' ) you go down a pile of tests like "This doesn't look like a selector, right?" "Does this looks like a single element, or a block of html?"
All just to express "Create a <div>". Which you get with document.createElement.
Frankly deal with the "document.createElement( 'div' )" is longer than "$( '<div />' )" and "I want to use jQuery methods on it." arguments I'd prefer having something like `$.tag( 'div' )`.
I still can't believe the high-level jQuery answer after all these years to "Select a div with an id provided by the user" is "Use `$( "div#" + userInput )` and hope there are no special characters. Or find some way to escape it yourself." when low-level dom can just query by ID and there is no reason for jQuery to force people to express everything in querys they parse when they could actually declare portions of a query with object notations.
But it's important to establish conventions which lead the developer down a path where they're less likely to run into trouble, even if they do try to take a few shortcuts. So let's add the slash.
(There is one important case where it's good to use innerHTML: when there are thousands of elements to create in a batch.)
-- Tim Starling
On 28/08/12 13:04, Daniel Friesen wrote:
I still can't believe the high-level jQuery answer after all these years to "Select a div with an id provided by the user" is "Use `$( "div#" + userInput )` and hope there are no special characters. Or find some way to escape it yourself." when low-level dom can just query by ID and there is no reason for jQuery to force people to express everything in querys they parse when they could actually declare portions of a query with object notations.
I share your reservations about jQuery, I voiced them at the time it was introduced to MediaWiki. I trolled the proponents by musing about how awesome jQuery would be if the selector engine (Sizzle) were removed.
Personally, I would use document.getElementById() to do that. It's standard, and it's faster and more secure. More complex selectors derived from user input can be replaced with jQuery.filter() etc. with no loss of performance.
-- Tim Starling
On 28 August 2012 09:57, Tim Starling tstarling@wikimedia.org wrote:
On 28/08/12 13:04, Daniel Friesen wrote:
I still can't believe the high-level jQuery answer after all these years to "Select a div with an id provided by the user" is "Use `$( "div#" + userInput )` and hope there are no special characters. Or find some way to escape it yourself." when low-level dom can just query by ID and there is no reason for jQuery to force people to express everything in querys they parse when they could actually declare portions of a query with object notations.
I share your reservations about jQuery, I voiced them at the time it was introduced to MediaWiki. I trolled the proponents by musing about how awesome jQuery would be if the selector engine (Sizzle) were removed.
Personally, I would use document.getElementById() to do that. It's standard, and it's faster and more secure. More complex selectors derived from user input can be replaced with jQuery.filter() etc. with no loss of performance.
The selector thing is a query language, and very powerful / abusable. Pretty much like SQL or any other 4th generation programming language.
Is high level, so you always have the risk of people doing something weird, but normally allow for JQuery programs to do in 3 lines of code what normally will take 30 or 50 lines. These 3 lines have less probability to be bug free, and shows intention better than the low level enhanced javascript code. The Javascript language is hard in a non obvious way, and this help is necessary.
On Tue, Aug 28, 2012 at 12:57 AM, Tim Starling tstarling@wikimedia.orgwrote:
Personally, I would use document.getElementById() to do that. It's standard, and it's faster and more secure.
I think your premature optimization disorder (POD) is flaring up again. jQuery performance is something that should be understood but not obsessed about. Is the code running in a tight loop? Is there a perceivable performance problem? No? Move on please.
$( '<div>' ) or $( '<div />' ) ?
I think it's fine to choose one for consistency, but both are equally performant[1]. and $( '<div>' ) has the benefit of being more brief.
Creating an element with attributes can be done like:
$( '<div>', { 'class', 'example' } );
It's important to note however that IE required that input and button tags are created with a type (if they are going to have a specific one)
$( '<input type="password">', { 'class', 'example' } );
- Trevor
On 28/08/12 19:11, Trevor Parscal wrote:
On Tue, Aug 28, 2012 at 12:57 AM, Tim Starling tstarling@wikimedia.orgwrote:
Personally, I would use document.getElementById() to do that. It's standard, and it's faster and more secure.
I think your premature optimization disorder (POD) is flaring up again. jQuery performance is something that should be understood but not obsessed about. Is the code running in a tight loop? Is there a perceivable performance problem? No? Move on please.
Don't feed the troll.
-- Tim Starling
Yes, $( '<div/>' ) or $( '<div></div>' ) is correct here, not $( '<div>' ). jQuery specifically states this in their documentation, and you'll run into problems in IE if you use the other form (as Neil discovered while writing UploadWizard). Not sure why this is flame-war worthy. Seems pretty straightforward to me, especially if you're in the habit of writing XHTML.
Ryan Kaldari
On 8/27/12 4:26 PM, Daniel Friesen wrote:
I ran into our coding conventions for creating elements in JS: https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Creating...
var $hello = $('<div>').text( 'Hello' ); // Not '<div/>' // Not '<div></div>'
This looks like some really bad advice.
This dates back to an issue I ran into with jQuery 3 years ago: https://forum.jquery.com/topic/ie-issue-with-append#14737000000469433 https://forum.jquery.com/topic/ie-issue-with-append#14737000000469445 Basically $( '<span class="foo">' ) will break completely in IE7/IE8.
jQuery supports /> on all elements (eg: $( '<span class="foo" />' )) intentionally. It does internal replacements to support it as a syntax for specifying elements. But besides that special case jQuery wants anything passed to it to be valid markup. It just leaves the parsing of it up to the browser and all the quirks the browser may have. jQuery does special case attribute-less $( '<div />' ) but this is a performance enhancement. The fact that $( '<div>' ) does not break in IE7/IE8 is an unintentional side effect of jQuery's lazy support of special cases like $( '<img>' ) where the tag is self closing and the browser will not require a /. This is the ONLY case where jQuery intentionally supports leaving out a closing tag or a self-closing /.
When devs consider `$( '<div>' )` ok they typically believe that `$( '<div class="foo">' )` should be ok to. It looks like a special cased way to define an element, attributes et. all. However the former is a performance enhancement side effect, and the later while it will look like it works in Chrome and Firefox actually relies entirely on quirky error correction behavior which differs between engines and breaks in IE7/IE8 engine.
The jQuery documentation also does not state that `$( '<div>' )` for non-selfclosing elements is supported: http://api.jquery.com/jQuery/#jQuery2
Hence, I think we should change our coding conventions to always use `$( '<div />' )`.
((And for anyone that suggests that developers should know they should add a / or </div> to <div> when they add any attributes to it. I bring up the fact that our code style requires {} blocks and does not allow implicit `if ( foo ) foo();`. This is the same thing.))
I knew there was a good reason I use $( '<div/>' ) One of those things I learn then forget why I'm doing it. I've apparently not being following styling guidelines ;-)
This is not flame war worthy. This is not about performance it simply causes problems in IE as Daniel points out. Let's just change the documentation to recommend $( '<div/>' ) and close this discussion or risk my wrath of restarting an inline styles conversation :P
On Tue, Aug 28, 2012 at 8:55 AM, Ryan Kaldari rkaldari@wikimedia.orgwrote:
Yes, $( '<div/>' ) or $( '<div></div>' ) is correct here, not $( '<div>' ). jQuery specifically states this in their documentation, and you'll run into problems in IE if you use the other form (as Neil discovered while writing UploadWizard). Not sure why this is flame-war worthy. Seems pretty straightforward to me, especially if you're in the habit of writing XHTML.
Ryan Kaldari
On 8/27/12 4:26 PM, Daniel Friesen wrote:
I ran into our coding conventions for creating elements in JS: https://www.mediawiki.org/**wiki/Manual:Coding_**conventions/JavaScript#* *Creating_elementshttps://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Creating_elements var $hello = $('<div>').text( 'Hello' ); // Not '<div/>' // Not '<div></div>'
This looks like some really bad advice.
This dates back to an issue I ran into with jQuery 3 years ago: https://forum.jquery.com/**topic/ie-issue-with-append#**14737000000469433https://forum.jquery.com/topic/ie-issue-with-append#14737000000469433 https://forum.jquery.com/**topic/ie-issue-with-append#**14737000000469445https://forum.jquery.com/topic/ie-issue-with-append#14737000000469445 Basically $( '<span class="foo">' ) will break completely in IE7/IE8.
jQuery supports /> on all elements (eg: $( '<span class="foo" />' )) intentionally. It does internal replacements to support it as a syntax for specifying elements. But besides that special case jQuery wants anything passed to it to be valid markup. It just leaves the parsing of it up to the browser and all the quirks the browser may have. jQuery does special case attribute-less $( '<div />' ) but this is a performance enhancement. The fact that $( '<div>' ) does not break in IE7/IE8 is an unintentional side effect of jQuery's lazy support of special cases like $( '<img>' ) where the tag is self closing and the browser will not require a /. This is the ONLY case where jQuery intentionally supports leaving out a closing tag or a self-closing /.
When devs consider `$( '<div>' )` ok they typically believe that `$( '<div class="foo">' )` should be ok to. It looks like a special cased way to define an element, attributes et. all. However the former is a performance enhancement side effect, and the later while it will look like it works in Chrome and Firefox actually relies entirely on quirky error correction behavior which differs between engines and breaks in IE7/IE8 engine.
The jQuery documentation also does not state that `$( '<div>' )` for non-selfclosing elements is supported: http://api.jquery.com/jQuery/#**jQuery2http://api.jquery.com/jQuery/#jQuery2
Hence, I think we should change our coding conventions to always use `$( '<div />' )`.
((And for anyone that suggests that developers should know they should add a / or </div> to <div> when they add any attributes to it. I bring up the fact that our code style requires {} blocks and does not allow implicit `if ( foo ) foo();`. This is the same thing.))
______________________________**_________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/**mailman/listinfo/wikitech-lhttps://lists.wikimedia.org/mailman/listinfo/wikitech-l
On Tue, Aug 28, 2012 at 10:06 AM, Jon Robson jdlrobson@gmail.com wrote:
I knew there was a good reason I use $( '<div/>' ) One of those things I learn then forget why I'm doing it. I've apparently not being following styling guidelines ;-)
Actually you were following the guidelines, but they were changed on as of 16:24, 28 August 2012 by Dantman:
https://www.mediawiki.org/w/index.php?title=Manual%3ACoding_conventions%2FJa...
Previously they looked like this:
https://www.mediawiki.org/w/index.php?title=Manual:Coding_conventions/JavaSc...
- Trevor
On 28 August 2012 02:26, Daniel Friesen daniel@nadir-seen-fire.com wrote:
I ran into our coding conventions for creating elements in JS: https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Creating... var $hello = $('<div>').text( 'Hello' ); // Not '<div/>' // Not '<div></div>'
This looks like some really bad advice.
The very reason I and others have been killing '<div/>' with fire is that I was told it doesn't work in IE. Can you explain what is going on?
Personally I don't like '<div/>' because there is no such thing in html. I could understand '<div>' as a shortcut for creating a div element.
Unless the issue is clarified, I think that recommending to always use valid html like <div></div> seems a much better option than either of <div> or <div/>. -Niklas
On Tue, 28 Aug 2012 10:40:23 -0700, Niklas Laxström niklas.laxstrom@gmail.com wrote:
On 28 August 2012 02:26, Daniel Friesen daniel@nadir-seen-fire.com wrote:
I ran into our coding conventions for creating elements in JS: https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Creating... var $hello = $('<div>').text( 'Hello' ); // Not '<div/>' // Not '<div></div>'
This looks like some really bad advice.
The very reason I and others have been killing '<div/>' with fire is that I was told it doesn't work in IE. Can you explain what is going on?
If they're talking about jQuery that's a mistake on part of whoever told you that.
jQuery has a regexp internally that converts <tag ... /> anywhere in the input automatically into <tag ...></tag>. It's an intentional shortcut so that you don't have to specify the closing tag every time you're creating an element that you're going to add content to dynamically.
Personally I don't like '<div/>' because there is no such thing in html. I could understand '<div>' as a shortcut for creating a div element.
Unless the issue is clarified, I think that recommending to always use valid html like <div></div> seems a much better option than either of
<div> or <div/>.
What would you think about bypassing all the pointless vs. discussion by adding something like `$.tag( 'div' )` or `$.create( 'div' );`?
-Niklas
wikitech-l@lists.wikimedia.org