Skip to content
Permalink
Browse files
Revert back to always setting the attribute to empty string before re… 
…moval. Fixes #9699.
  • Loading branch information
@timmywil
timmywil committed Aug 25, 2011
1 parent e570577 commit d723942b274e3e48dac82ebde11906a4cb349415
Showing 2 changed files with 12 additions and 16 deletions.
11 src/attributes.js
View file
@@ -366,14 +366,9 @@ jQuery.extend({
var propName;
if ( elem.nodeType === 1 ) {
name = jQuery.attrFix[ name ] || name;

if ( jQuery.support.getSetAttribute ) {
// Use removeAttribute in browsers that support it
elem.removeAttribute( name );
} else {
jQuery.attr( elem, name, "" );
elem.removeAttributeNode( elem.getAttributeNode( name ) );
}

jQuery.attr( elem, name, "" );
elem.removeAttribute( name );

// Set corresponding property to false for boolean attributes
if ( rboolean.test( name ) && (propName = jQuery.propFix[ name ] || name) in elem ) {
17 test/unit/attributes.js
View file
@@ -451,17 +451,18 @@ test("attr('tabindex', value)", function() {
});

test("removeAttr(String)", function() {
expect(7);
equals( jQuery("#mark").removeAttr( "class" )[0].className, "", "remove class" );
equals( jQuery("#form").removeAttr("id").attr("id"), undefined, "Remove id" );
equals( jQuery("#foo").attr("style", "position:absolute;").removeAttr("style").attr("style"), undefined, "Check removing style attribute" );
equals( jQuery("#form").attr("style", "position:absolute;").removeAttr("style").attr("style"), undefined, "Check removing style attribute on a form" );
equals( jQuery("#fx-test-group").attr("height", "3px").removeAttr("height").css("height"), "1px", "Removing height attribute has no effect on height set with style attribute" );
expect(8);
equal( jQuery("#mark").removeAttr( "class" )[0].className, "", "remove class" );
equal( jQuery("#form").removeAttr("id").attr("id"), undefined, "Remove id" );
equal( jQuery("#foo").attr("style", "position:absolute;").removeAttr("style").attr("style"), undefined, "Check removing style attribute" );
equal( jQuery("#form").attr("style", "position:absolute;").removeAttr("style").attr("style"), undefined, "Check removing style attribute on a form" );
equal( jQuery("<div style='position: absolute'></div>").appendTo("#foo").removeAttr("style").prop("style").cssText, "", "Check removing style attribute (#9699 Webkit)" );
equal( jQuery("#fx-test-group").attr("height", "3px").removeAttr("height").css("height"), "1px", "Removing height attribute has no effect on height set with style attribute" );

jQuery("#check1").removeAttr("checked").prop("checked", true).removeAttr("checked");
equals( document.getElementById("check1").checked, false, "removeAttr sets boolean properties to false" );
equal( document.getElementById("check1").checked, false, "removeAttr sets boolean properties to false" );
jQuery("#text1").prop("readOnly", true).removeAttr("readonly");
equals( document.getElementById("text1").readOnly, false, "removeAttr sets boolean properties to false" );
equal( document.getElementById("text1").readOnly, false, "removeAttr sets boolean properties to false" );
});

test("prop(String, Object)", function() {

9 comments on commit d723942

@timmywil
Copy link
Member Author

@timmywil timmywil commented on d723942 Aug 25, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit also fixes #9719

@martinr92
Copy link

@martinr92 martinr92 commented on d723942 Aug 26, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@tim-evans
Copy link

@tim-evans tim-evans commented on d723942 Dec 19, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit seems to break SproutCore SC.TextFieldView in IE. Could you explain a bit more how this commit changes IE functionality so we can work to upgrade SproutCore to use jQuery >1.6.2?

@timmywil
Copy link
Member Author

@timmywil timmywil commented on d723942 Dec 19, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tim-evans: can you be more specific about the broken behavior?

@tim-evans
Copy link

@tim-evans tim-evans commented on d723942 Dec 19, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timmywil the fields won't accept focus. You can make text range selections, but no cursor ever shows up in the field.

@timmywil
Copy link
Member Author

@timmywil timmywil commented on d723942 Dec 19, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tim-evans: Which attribute is being removed?

@tim-evans
Copy link

@tim-evans tim-evans commented on d723942 Dec 19, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timmywil on the actual <input/>, only placeholder (which in my case, is not being changed). A parent element gets tabindex, changed when the text field is focused.

@timmywil
Copy link
Member Author

@timmywil timmywil commented on d723942 Dec 19, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit only affects removing an attribute, but until this commit: f2c1d2e, setting any camelCase boolean attributes in IE6/7 to empty string caused it to be added to the html (effectively setting the property to true) and then not removing it with removeAttribute. That's the only problem I can think of right now.

@tim-evans
Copy link

@tim-evans tim-evans commented on d723942 Dec 19, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this affects IE8/9, so that's probably not what's causing our issue. I'll try to make up a test case, or at least put up a demo app where you can take a peek at the bug.

Please sign in to comment.