Bug Tracker

Ticket #787 (closed bug: fixed)

Opened 8 years ago

Last modified 7 years ago

Array support in selector is broken

Reported by: Fil Owned by:
Priority: major Milestone: 1.1
Component: core Version: 1.1a
Keywords: array selector Cc: fil@…
Blocking: Blocked by:

Description

Here's a test that shouldn't be broken

test("expressions - array", function() {
	expect(1);
	t( "Array Support", ["a.blog", "div"], ["mark","simon","main","foo"] );
});

Attachments

787.patch Download (1.3 KB) - added by Fil 8 years ago.
test and fix for this bug ; .substring() is also used for speed
787.2.patch Download (1.3 KB) - added by Fil 8 years ago.
test and fix for this bug ; .substring() is also used for speed
787.patch.txt Download (1.3 KB) - added by Fil 8 years ago.
test and fix for this bug ; .substring() is also used for speed

Change History

Changed 8 years ago by Fil

test and fix for this bug ; .substring() is also used for speed

Changed 8 years ago by Fil

test and fix for this bug ; .substring() is also used for speed

Changed 8 years ago by Fil

test and fix for this bug ; .substring() is also used for speed

comment:1 Changed 8 years ago by Fil

trac doesn't let me attach the patch, so here it is (ugly)

Index: src/selector/selector.js
===================================================================
--- src/selector/selector.js	(revision 1016)
+++ src/selector/selector.js	(working copy)
@@ -297,6 +297,10 @@
 	},
 
 	filter: function(t,r,not) {
+		// Support for array selectors
+		if ( typeof t == 'object' )
+			t = t.join(',');
+
 		// Look for common filter expressions
 		while ( t && /^[a-z[({<*:.#]/i.test(t) ) {
 
@@ -312,13 +316,13 @@
 				var m = re.exec( t );
 
 				if ( m ) {
+					// Remove what we just matched
+					t = t.substring( m[0].length );
+
 					// Re-organize the first match
 					if ( jQuery.expr[ m[1] ]._resort )
 						m = jQuery.expr[ m[1] ]._resort( m );
 
-					// Remove what we just matched
-					t = t.replace( re, "" );
-
 					break;
 				}
 			}
Index: src/selector/selectorTest.js
===================================================================
--- src/selector/selectorTest.js	(revision 1016)
+++ src/selector/selectorTest.js	(working copy)
@@ -41,6 +41,12 @@
 	t( "Comma Support", "a.blog,div", ["mark","simon","main","foo"] );

 });

 

+test("expressions - array", function() {

+	expect(1);

+	t( "Array Support", ["a.blog", "div"], ["mark","simon","main","foo"] );

+});

+

+

 test("expressions - child and adjacent", function() {

 	expect(14);

 	t( "Child", "p > a", ["simon1","google","groups","mark","yahoo","simon"] );


comment:2 Changed 8 years ago by Fil

I don't know if the array selector is going to be of type "object" on all browsers. It works on FF2/Mac and Safari though.

Please note that I included a .substring() for speed.

comment:3 Changed 8 years ago by Fil

Or, if the typeof test is costly, you could remove support for the array selector, the comma separated stuff is enough. But please mention it in the changelog :)

comment:4 Changed 8 years ago by john

  • Status changed from new to closed
  • Resolution set to fixed

We're removing support for the array selector in favor of the comma separated one. I mentioned this at the end of the recent blog post and there's support for it in the compatibility plugin. I'll be sure to re-announce it when its released.

I also added in your substring improvement.

comment:5 Changed 8 years ago by Fil

Good. I hadn't seen it in the blog post :)

Note: See TracTickets for help on using tickets.