Permalink
Browse files

Remove leading/trailing text from $(html). Close gh-27.

  • Loading branch information...
1 parent 514a24b commit c8147a3ba66cc50a05ae09ffa3d3f3df811f30ae @dmethvin dmethvin committed Mar 28, 2013
Showing with 17 additions and 4 deletions.
  1. +6 −1 src/core.js
  2. +5 −3 test/core.js
  3. +6 −0 warnings.md
View
@@ -2,6 +2,7 @@
var matched, browser,
oldInit = jQuery.fn.init,
oldParseJSON = jQuery.parseJSON,
+ rignoreText = /^[^<]*(.*?)[^>]*$/,
// Note this does NOT include the #9521 XSS fix from 1.7!
rquickExpr = /^(?:[^<]*(<[\w\W]+>)[^>]*|#([\w\-]*))$/;
@@ -15,13 +16,17 @@ jQuery.fn.init = function( selector, context, rootjQuery ) {
if ( selector.charAt( 0 ) !== "<" ) {
migrateWarn("$(html) HTML strings must start with '<' character");
}
+ if ( selector.charAt( selector.length -1 ) !== ">" ) {
+ migrateWarn("$(html) HTML string has stray text after last tag");
+ }
// Now process using loose rules; let pre-1.8 play too
if ( context && context.context ) {
// jQuery object as context; parseHTML expects a DOM object
context = context.context;
}
if ( jQuery.parseHTML ) {
- return oldInit.call( this, jQuery.parseHTML( jQuery.trim(selector), context, true ),
+ match = rignoreText.exec( selector );
+ return oldInit.call( this, jQuery.parseHTML( match[1] || selector, context, true ),
context, rootjQuery );
}
}
View
@@ -12,7 +12,7 @@ test( "jQuery(html, props)", function() {
});
test( "jQuery(html) loose rules", function() {
- expect( 19 );
+ expect( 25 );
var w,
nowarns = {
@@ -23,11 +23,13 @@ test( "jQuery(html) loose rules", function() {
warns = {
"leading space": " <div />",
"leading newline": "\n<div />",
- "leading text": "don't<div>try this</div>"
+ "leading text": "don't<div>try this</div>",
+ "trailing text": "<div>try this</div>junk",
+ "both text": "don't<div>try this</div>either"
},
generate = function( html ) {
return function() {
- var el = jQuery( html ).last();
+ var el = jQuery( html );
equal( el.length, 1, html + " succeeded" );
equal( el.parent().length, 0, html + " generated new content" );
View
@@ -24,6 +24,12 @@ This is _not_ a warning, but a console log message the plugin shows when it firs
**Solution**: Use the `$.parseHTML()` method to parse arbitrary HTML, especially HTML from external sources. To obtain a jQuery object that has the parsed HTML without running scripts, use `$($.parseHTML("string"))`. To run scripts in the HTML as well, use `$($.parseHTML("string", document, true))` instead. We do not recommend running `$.trim()` on the string to circumvent this check.
+### JQMIGRATE: $(html) text after last tag is ignored
+
+**Cause:** HTML strings passed to `$()` should begin and end with tags. Any text following the last tag is ignored. When upgrading to jQuery 1.9 and using `$.parseHTML()`, note that leading or trailing text is _not_ ignored, and those text nodes will be part of the data returned.
+
+**Solution**: Usually this warning is due to an error in the HTML string, where text is present when it should not be there. Remove the leading or trailing text before passing the string to `$.parseHTML()` if it should not be part of the collection. Alternatively you can use `$($.parseHTML(html)).filter("*")` to remove all top-level text nodes from the set and leave only elements.
+
### JQMIGRATE: Can't change the 'type' of an input or button in IE 6/7/8
**Cause:** IE 6, 7, and 8 throw an error if you attempt to change the type attribute of an input or button element, for example to change a radio button to a checkbox. Prior to 1.9, jQuery threw an error for every browser to create consistent behavior. As of jQuery 1.9 setting the type is allowed, but will still throw an error in oldIE.

3 comments on commit c8147a3

Owner
gnarf commented on c8147a3 Mar 28, 2013

Is this test case one for jQuery core as well, do we already test this?

Owner

In core we just ignore any leading/trailing text and throw it away, that's been the behavior forever. The point here is that if you take $("xxx<div>abc</div>yyy") and turn it into $.parseHTML("xxx<div>abc</div>yyy") you'll get different results. See the original bug report for some background, this bit the WordPress folks. So it's mainly a "are you sure this is right?" kind of thing.

Owner

I was just wondering if we actually TEST that in core, or if we just leave it up to that's the way it works.

Please sign in to comment.