Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix #14074: element id="nodeName" #1389

Closed
wants to merge 3 commits into from

4 participants

Richard Gibson Dave Methvin Rick Waldron Michał Gołębiowski
Richard Gibson
Collaborator

#1373 seems to have been abandoned, so here's an alternative.

   raw     gz Compared to 1.x-master @ 94a9a4f1c2db7b80111fe24afc46de7c303f5a29
   -13    +14 dist/jquery.js
   -18     +8 dist/jquery.min.js
src/data.js
((8 lines not shown))
// Ban all objects except for Flash (which handle expandos)
- "object": "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000"
+ "object ": "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000"
Rick Waldron Collaborator
rwaldron added a note

@gibson042 could you include a explanation comment? Thanks!

Dave Methvin Owner
dmethvin added a note

@rwaldron about the use of the trailing space, or the clsid? The comment above does the latter (well when combined with the conditional below).

Richard Gibson Collaborator

I updated to explain both a little better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Rick Waldron rwaldron commented on the diff
test/data/core/aliased.html
((1 lines not shown))
+<!doctype html>
+<html>
+<head>
+ <meta http-equiv="Content-type" content="text/html; charset=utf-8">
+ <title>alias-masked DOM properties (#14074)</title>
+ <script>
+ var errors = [];
+ window.onerror = function( errorMessage, filePath, lineNumber ) {
+ errors.push( errorMessage );
+ };
+ </script>
+ <script src="../../jquery.js"></script>
+</head>
+<body>
+ <form>
+ <input type="text" id="nodeName"/>
Rick Waldron Collaborator
rwaldron added a note

/> is unnecessary with this doctype ;)

Dave Methvin Owner
dmethvin added a note

Probably good to keep it though, since we may want to do do XHTML tests and would want to keep the markup compatible with both.

Richard Gibson Collaborator

Good point @dmethvin; I fixed up the meta tag too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Dave Methvin dmethvin referenced this pull request
Closed

Fix for bug #14074 #1373

Dave Methvin
Owner

Yeah this looks a little more bulletproof.

Rick Waldron rwaldron commented on the diff
src/data.js
((8 lines not shown))
noData: {
- "applet": true,
- "embed": true,
- // Ban all objects except for Flash (which handle expandos)
- "object": "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000"
+ "applet ": true,
+ "embed ": true,
+ // ...but Flash objects (which have this classid) *can* handle expandos
+ "object ": "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000"
Rick Waldron Collaborator
rwaldron added a note

@dmethvin I don't know where your comment went, but in response, the latter I'm very familiar with, it was the trailing spaces in the property names that I'm curious about. I think we can all agree it's not the sort of thing you'd normally see, so I think a small comment explaining why is not a bad idea.

Michał Gołębiowski Collaborator
mzgol added a note

+1 for the comment, at first glance it seems like a mistake so it'd be good to clarify what's happening here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Richard Gibson
Collaborator

I'd like to land this for the next beta. Does anyone object?

Dave Methvin
Owner

LGTM

Richard Gibson gibson042 closed this pull request from a commit
Richard Gibson gibson042 Fix #14074: element id="nodeName". Close gh-1389.
(cherry picked from commit 126d596)

Conflicts:

	src/data.js
	src/data/accepts.js
	test/unit/core.js
	test/unit/data.js
c66a5e7
Richard Gibson gibson042 closed this in c66a5e7
Mescoda mescoda referenced this pull request from a commit in mescoda/jquery
Richard Gibson gibson042 Fix #14074: element id="nodeName". Close gh-1389. 2588505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 19, 2013
  1. Richard Gibson
Commits on Oct 3, 2013
  1. Richard Gibson

    improve comments

    gibson042 authored
  2. Richard Gibson

    XHTML compatibility

    gibson042 authored
This page is out of date. Refresh to see the latest.
12 src/data.js
View
@@ -238,13 +238,13 @@ function internalRemoveData( elem, name, pvt ) {
jQuery.extend({
cache: {},
- // The following elements throw uncatchable exceptions if you
- // attempt to add expando properties to them.
+ // The following elements (space-suffixed to avoid Object.prototype collisions)
+ // throw uncatchable exceptions if you attempt to set expando properties
noData: {
- "applet": true,
- "embed": true,
- // Ban all objects except for Flash (which handle expandos)
- "object": "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000"
+ "applet ": true,
+ "embed ": true,
+ // ...but Flash objects (which have this classid) *can* handle expandos
+ "object ": "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000"
Rick Waldron Collaborator
rwaldron added a note

@dmethvin I don't know where your comment went, but in response, the latter I'm very familiar with, it was the trailing spaces in the property names that I'm curious about. I think we can all agree it's not the sort of thing you'd normally see, so I think a small comment explaining why is not a bad idea.

Michał Gołębiowski Collaborator
mzgol added a note

+1 for the comment, at first glance it seems like a mistake so it'd be good to clarify what's happening here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
},
hasData: function( elem ) {
14 src/data/accepts.js
View
@@ -6,15 +6,15 @@ define([
* Determines whether an object can have data
*/
jQuery.acceptData = function( elem ) {
- // Do not set data on non-element because it will not be cleared (#8335).
- if ( elem.nodeType && elem.nodeType !== 1 && elem.nodeType !== 9 ) {
- return false;
- }
+ var noData = jQuery.noData[ (elem.nodeName + " ").toLowerCase() ],
+ nodeType = +elem.nodeType || 1;
- var noData = elem.nodeName && jQuery.noData[ elem.nodeName.toLowerCase() ];
+ // Do not set data on non-element DOM nodes because it will not be cleared (#8335).
+ return nodeType !== 1 && nodeType !== 9 ?
+ false :
- // nodes accept data unless otherwise specified; rejection can be conditional
- return !noData || noData !== true && elem.getAttribute("classid") === noData;
+ // Nodes accept data unless otherwise specified; rejection can be conditional
+ !noData || noData !== true && elem.getAttribute("classid") === noData;
};
return jQuery.acceptData;
24 test/data/core/aliased.html
View
@@ -0,0 +1,24 @@
+<!doctype html>
+<html>
+<head>
+ <meta http-equiv="Content-type" content="text/html; charset=utf-8"/>
+ <title>alias-masked DOM properties (#14074)</title>
+ <script>
+ var errors = [];
+ window.onerror = function( errorMessage, filePath, lineNumber ) {
+ errors.push( errorMessage );
+ };
+ </script>
+ <script src="../../jquery.js"></script>
+</head>
+<body>
+ <form>
+ <input type="text" id="nodeName"/>
Rick Waldron Collaborator
rwaldron added a note

/> is unnecessary with this doctype ;)

Dave Methvin Owner
dmethvin added a note

Probably good to keep it though, since we may want to do do XHTML tests and would want to keep the markup compatible with both.

Richard Gibson Collaborator

Good point @dmethvin; I fixed up the meta tag too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ </form>
+ <script>
+ jQuery(function() {
+ window.parent.iframeCallback( errors );
+ });
+ </script>
+</body>
+</html>
7 test/unit/core.js
View
@@ -1419,3 +1419,10 @@ testIframeWithCallback( "Conditional compilation compatibility (#13274)", "core/
deepEqual( errors, [], "No errors" );
ok( $(), "jQuery executes" );
});
+
+testIframeWithCallback( "Tolerating alias-masked DOM properties (#14074)", "core/aliased.html",
+ function( errors ) {
+ expect( 1 );
+ deepEqual( errors, [], "jQuery loaded" );
+ }
+);
22 test/unit/data.js
View
@@ -142,26 +142,30 @@ test("Data is not being set on comment and text nodes", function() {
});
test("jQuery.acceptData", function() {
- expect(9);
+ expect( 10 );
var flash, applet;
ok( jQuery.acceptData( document ), "document" );
ok( jQuery.acceptData( document.documentElement ), "documentElement" );
ok( jQuery.acceptData( {} ), "object" );
- ok( !jQuery.acceptData( document.createElement("embed") ), "embed" );
- ok( !jQuery.acceptData( document.createElement("applet") ), "applet" );
+ ok( !jQuery.acceptData( document.createElement( "embed" ) ), "embed" );
+ ok( !jQuery.acceptData( document.createElement( "applet" ) ), "applet" );
- flash = document.createElement("object");
- flash.setAttribute("classid", "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000");
+ flash = document.createElement( "object" );
+ flash.setAttribute( "classid", "clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" );
ok( jQuery.acceptData( flash ), "flash" );
- applet = document.createElement("object");
- applet.setAttribute("classid", "clsid:8AD9C840-044E-11D1-B3E9-00805F499D93");
+ applet = document.createElement( "object" );
+ applet.setAttribute( "classid", "clsid:8AD9C840-044E-11D1-B3E9-00805F499D93" );
ok( !jQuery.acceptData( applet ), "applet" );
- ok( !jQuery.acceptData( document.createComment("") ), "comment" );
- ok( !jQuery.acceptData( document.createTextNode("") ), "text" );
+ ok( !jQuery.acceptData( document.createComment( "" ) ), "comment" );
+ ok( !jQuery.acceptData( document.createTextNode( "" ) ), "text" );
+
+ ok( jQuery.acceptData(
+ jQuery( "#form" ).append( "<input id='nodeType'/><input id='nodeName'/>" )[ 0 ] ),
+ "form with aliased DOM properties" );
});
test(".data()", function() {
Something went wrong with that request. Please try again.