Good catch - and thanks for the patch! I just landed the improvement:
http://dev.jquery.com/ticket/4884
I made one minor tweak to the patch: In the case where .attr("name",
function(){}) occurs the jQuery.isFunction(value) check is cached (rather
than occurring at every iteration in the array).
Thanks again!
--John
On Fri, Jul 10, 2009 at 7:17 PM, David Flanagan <da...@davidflanagan.com>wrote:
> The 1.3.3pre version of the attr() method doesn't handle object
> arguments where property values are functions in the same way that 1.3.2
> does (and the way that the docs say it should). I think because in the
> code reorganization the jQuery.prop() helper function went away. I
> believe that a very similar bug exists in the css() method, but I
> haven't looked closely enough at it.
>
> I can't figure out how to submit a bug report against 1.3.3pre, so I'm
> posting this here. Let me know if I should file it as a bug against
> some other version on the tracker.
>
> I've attached a patch to src/attributes.js and also to
> tests/unit/attributes.js
>
> In my patch I also try to optimze the (I assume common) case where
> attr() is called with two arguments. It no longer creates and then
> iterates over a new object in this case. But it does make the code
> slightly longer.
>
> David Flanagan
>
> >
>
> Index: test/unit/attributes.js
> ===================================================================
> --- test/unit/attributes.js (revision 6415)
> +++ test/unit/attributes.js (working copy)
> @@ -68,12 +68,15 @@
> });
>
> test("attr(Hash)", function() {
> - expect(1);
> + expect(3);
> var pass = true;
> jQuery("div").attr({foo: 'baz', zoo: 'ping'}).each(function(){
> if ( this.getAttribute('foo') != "baz" &&
> this.getAttribute('zoo') != "ping" ) pass = false;
> });
> ok( pass, "Set Multiple Attributes" );
> + equals( jQuery('#text1').attr({'value': function() { return
> this.id; }})[0].value, "text1", "Set attribute to computed value #1" );
> + equals( jQuery('#text1').attr({'title': function(i) { return i;
> }}).attr('title'), "0", "Set attribute to computed value #2");
> +
> });
>
> test("attr(String, Object)", function() {
> @@ -334,9 +337,9 @@
> e.toggleClass(false);
> e.toggleClass();
> ok( e.is(".testD.testE"), "Assert class present (restored from
> data)" );
> -
> -
>
> +
> +
> // Cleanup
> e.removeClass("testD");
> e.removeData('__className__');
> Index: src/attributes.js
> ===================================================================
> --- src/attributes.js (revision 6415)
> +++ src/attributes.js (working copy)
> @@ -1,34 +1,30 @@
> jQuery.fn.extend({
> attr: function( name, value ) {
> - var options = name, isFunction = jQuery.isFunction( value
> );
> + var elem, options;
>
> - if ( typeof name === "string" ) {
> - // Are we setting the attribute?
> - if ( value === undefined ) {
> + if ( typeof name === "string" ) { // A single attribute
> + if ( value === undefined ) { // Query it on
> first element
> return this.length ?
> jQuery.attr( this[0], name ) :
> null;
> -
> - // Convert name, value params to options hash
> format
> - } else {
> - options = {};
> - options[ name ] = value;
> + } else { // Set it on
> all elements
> + for ( var i = 0, l = this.length; i < l;
> i++ ) {
> + elem = this[i];
> + if ( jQuery.isFunction(value) )
> + value = value.call(elem,i);
> + jQuery.attr( elem, name, value );
> + }
> }
> - }
> -
> - // For each element...
> - for ( var i = 0, l = this.length; i < l; i++ ) {
> - var elem = this[i];
> -
> - // Set all the attributes
> - for ( var prop in options ) {
> - value = options[prop];
> -
> - if ( isFunction ) {
> - value = value.call( elem, i );
> + } else { // Multiple
> attributes to set on all
> + options = name;
> + for ( var i = 0, l = this.length; i < l; i++ ) {
> + elem = this[i];
> + for ( name in options ) {
> + value = options[name];
> + if ( jQuery.isFunction(value) )
> + value = value.call(elem,i);
> + jQuery.attr( elem, name, value );
> }
> -
> - jQuery.attr( elem, prop, value );
> }
> }
>
> @@ -258,4 +254,4 @@
> // Using attr for specific style information is now
> deprecated. Use style insead.
> return jQuery.style(elem, name, value);
> }
> -});
> \ No newline at end of file
> +});
>
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"jQuery Development" group.
To post to this group, send email to jquery-dev@googlegroups.com
To unsubscribe from this group, send email to
jquery-dev+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/jquery-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
