#7824 closed bug (notabug)

Opened October 29, 2011 10:18PM UTC

Closed October 29, 2011 10:41PM UTC

Last modified October 30, 2011 11:53AM UTC

el.addClass can't be passed to $.each

Reported by: TALlama Owned by:
Priority: minor Milestone: 1.9.0
Component: ui.effects.core Version: 1.8.16
Keywords: Cc:
Blocked by: Blocking:
Description

See the unit tests below. Note that commenting out jQuery UI will cause both tests to pass; it's the jQuery UI override to $.fn.addClass that is causing the failure.

<!DOCTYPE html>
<html lang="en">
  <head>
  	<title>QUnit Tests</title>
  	<link rel="stylesheet" href="qunit/qunit.css" media="screen">

    <!-- reference your own javascript files here -->
    <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.6.4/jquery.min.js"></script> 
    <script src="https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.16/jquery-ui.min.js"></script>


    <!-- test runner files -->
    <script src="qunit/qunit.js"></script>
    <script>
      test("Passing the function in", 0, function() {
        var p = $("<p>");
        $.each(['x'], p.addClass);
      });
      test("Called with this", 0, function() {
        var p = $("<p>");
        $.each(['x'], function() {p.addClass(this)});
      });
    </script>


  </head>
  <body class="flora">
  	<h1 id="qunit-header">QUnit Test Suite</h1>
  	<h2 id="qunit-banner"></h2>
  	<div id="qunit-testrunner-toolbar"></div>
  	<h2 id="qunit-userAgent"></h2>
  	<ol id="qunit-tests"></ol>
  	<div id="qunit-fixture">test markup</div>
  </body>
</html>
Attachments (0)
Change History (5)

Changed October 29, 2011 10:30PM UTC by gnarf comment:1

component: ui.core → effects.core

Changed October 29, 2011 10:41PM UTC by gnarf comment:2

resolution: → invalid
status: new → closed

The first test is not valid code... You lose the context of 

p
. seems like a completely invalid use of addClass to me...

Second use is also not valid, jQuery 's each can't pass a native string as 

 this
. You should use the arguments.

$.each(['x'], function( index, str ) {
  p.addClass( str );
});

However, if you have an array of strings, you could just join them with a space:

p.addClass( ["x", "y", "z"].join( " " ) );

Changed October 30, 2011 05:38AM UTC by TALlama comment:3

That would be a fine answer if the code didn't work when jQuery UI wasn't included, but this is code that works with straight jQuery and not with jQuery UI. You've taken code that works normally and made it so it doesn't work when you're there. I know that there's a workaround; I included one in my test case, for crying out loud. But you still shouldn't be breaking things that work if you can at all help it.

I also how I have no option to reopen the ticket…

Changed October 30, 2011 08:38AM UTC by gf3 comment:4

Both of the original code samples are incorrect Javascript for the desired outcome. The first sample would almost be possible with $.proxy, however the arguments that the function would receive are not simply a class name.

The second code sample is possible by simply casting to a string:

    var p = $("<p>");
    $.each(['x'], function() { p.addClass( this.toString() ) });

Note that **gnarf**'s final suggestion is a better solution.

Changed October 30, 2011 11:53AM UTC by scottgonzalez comment:5

Replying to [comment:3 TALlama]:

That would be a fine answer if the code didn't work when jQuery UI wasn't included, but this is code that works with straight jQuery and not with jQuery UI.

You seem to be missing gnarf's point. Regardless of whether it works, you're doing something that isn't supported by jQuery. This is an invalid use of jQuery and can stop working at any time.