Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#7824 closed bug (notabug)

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>

Change History (5)

comment:1 Changed 8 years ago by Corey Frang

Component: ui.coreeffects.core

comment:2 Changed 8 years ago by Corey Frang

Resolution: invalid
Status: newclosed

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( " " ) );

comment:3 Changed 8 years ago by 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'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…

comment:4 Changed 8 years ago by gf3

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.

comment:5 in reply to:  3 Changed 8 years ago by Scott González

Replying to 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.

Note: See TracTickets for help on using tickets.