jquery/jquery — pull request #886 by elijahmanor (2012-08-08 20:38:51+00:00)
Title: Prevent Clearing Cloned Style to Affect Source Element's Style. Fixes #8908

# Clearing a Cloned Element's Style Causes the Original Element's Style to be Cleared

## Introduction

In IE9 and IE10 you can run into the situation when clearing (`""`) a cloned element's style will also clear out the original element's corresponding style.

The actual [jQuery Core Ticket #8908](http://bugs.jquery.com/ticket/8908) isolates the bug when using the background-image style.

While I was trying to recreate and research this issue I had a feeling that there may be other styles that have the same issue, so I've created a [test suite](https://gist.github.com/b12e402ab0c25b0c8b02) checking [252 CSS1-3 styles](http://meiert.com/en/indices/css-properties/) and testing various assertions.

Along the way, I also found some additional inconsistencies. These findings are technically unrelated to the #8908 issue so I will list those inconsistencies somewhere else and we can discuss those later.

### Source Element's Style Cleared When Cloned Element's Style Cleared

### IE9 & IE10

After testing 252 various styles only the following 8 ended up being problematic.

* background-attachment
* background-color
* background-image
* background-position
* background-repeat
* background-clip
* background-origin
* background-size

### IE 6/7/8, Chrome, Firefox, Opera

Thankfully these browsers did not have the problem described in this issue when tested.

## Proposed Solutions

### 1. Use Default Values when Trying to Empty Style

The following fix resolves the weird issue by substituting and empty string with it's default value. An object is used to store the style name and associated default value. If the value happens to be an empty string then it will look in the object for a matching key and if there is a match it will use that instead.

```diff
// css.js
+ // These styles were identified to be problems in IE9/10
+ var defaultValue = {
+ "background-attachment": "scroll",
+ "background-color": "transparent",
+ "background-image": "none",
+ "background-position": "0% 0%",
+ "background-repeat": "repeat",
+ "background-clip": "border-box",
+ "background-origin": "padding-box",
+ "background-size": "auto auto"
+ };

+ value = ( value === "" && defaultValue[ name ] !== undefined ) ? defaultValue[ name ] : value;

try {
style[ name ] = value;
} catch(e) {}
```

#### Pros

* This is very explicit in describing which styles are problematic in IE

#### Cons

* There is extra code executed when clearing the style for a nonIE browser. This could be a problem because updating a style via the `.css()` method is very common
* There is a considerable amount of code to account for all problematic styles and this will bloat the jQuery source

### 2. Strange Side Effect That Breaks the Clone Connection

This solution is a little more strange and not so obvious. I ran into this fix as I was building up my unit tests and asserting a bunch of things I thought should be true. One of those assertions was verifying that when setting a style using the `$elem.css( "someStyle", "someValue" )` method that it actually worked. So, I used the getter (`$elem.css( "someValue" )` method, but by doing so I found the problem going away just by calling the getter! And with that the following solution came about.

You may notice a slightly strange piece of code `( window.getComputedStyle( elem, null ) || {} )`, but that was needed to get around an Opera bug I had when working with XML nodes. Technically I could have made the whole fix into one statement, but the minification process does a decent job of squishing down the fix to look like `(b.nodeType===1&&a.getComputedStyle&&(i=(a.getComputedStyle(b,null)||{}).backgroundImage)`.

```diff
// clone.js
jQuery.extend({
clone: function( elem, dataAndEvents, deepDataAndEvents ) {
var srcElements,
destElements,
i,
- clone;
+ clone,
+ fix8908;

if ( jQuery.support.html5Clone || jQuery.isXMLDoc(elem) || !rnoshimcache.test( "<" + elem.nodeName + ">" ) ) {
+ // Fixes #8909 - By accessing one of the element's computed styles it breaks the
+ // connection with the cloned element's styles in IE9/10
+ if ( elem.nodeType === 1 && window.getComputedStyle ) {
+ fix8908 = ( window.getComputedStyle( elem, null ) || {} ).backgroundImage;
+ }
+
clone = elem.cloneNode( true );
// IE<=8 does not properly clone detached, unknown element nodes
} else {
...
}
...
},
...
});
```

#### Pros

* The amount of code for this fix is relatively small
* This code is limited to the clone execution path, which happens infrequently when compared to the `.css()` method approach in option #1.

#### Cons

* This relies on a weird quirk in IE9/10 to get around the problem, but it seems to be the most succinct.

### 3. Have IE9 & IE10 Fix this Bug

It would be ideal if no patch had to be made to jQuery at all and IE9 and IE10 could apply a fix instead. Seeing that the issue is found in IE9 though may rule out this option entirely.

#### Pros

* No patch for jQuery would be needed

#### Cons

* Even if the bug was fixed immediately for IE9 & IE10 there would still be an issue with versions of IE9 that didn't have the IE fix.

## Unit Test

The following Unit Test exposes the problem in IE9 and IE10. In browsers that don't support these styles I am skipping over them and asserting that they are not an issue.

```javascript
test( "Clearing a Cloned Element's Style Shouldn't Clear the Original Element's Style (#8908)", function() {
expect( 8 );

var baseUrl = document.location.href.replace( /([^\/]*)$/, "" );
var styles = [
{ name: "background-attachment", value: [ "fixed" ], expected: [ "scroll" ] },
{ name: "background-color", value: [ "rgb(255, 0, 0)", "rgb(255,0,0)" ], expected: [ "transparent" ] },
{ name: "background-image", value: [ 'url("test.png")', 'url(' + baseUrl + 'test.png)', 'url("' + baseUrl + 'test.png")' ], expected: [ "none" ] },
{ name: "background-position", value: [ "5% 5%" ], expected: [ "0% 0%" ] },
{ name: "background-repeat", value: [ "repeat-y" ], expected: [ "repeat" ] },
{ name: "background-clip", value: [ "content-box" ], expected: [ "border-box" ] },
{ name: "background-origin", value: [ "content-box" ], expected: [ "padding-box" ] },
{ name: "background-size", value: [ "80px 60px" ], expected: [] }
];

jQuery.each( styles, function(index, style) {
var $source, source, $clone;

style.expected = style.expected.concat( [ "", "auto" ] );
$source = jQuery( "

" );
source = $source[ 0 ];
if ( source.style[ style.name ] === undefined ) {
ok( true, style.name + ": style isn't supported and therefore not an issue" );
return true;
}
$source.css( style.name, style.value[0] );
$clone = $source.clone();
$clone.css( style.name, "" );

ok( ~jQuery.inArray( $source.css( style.name ), style.value ),
"Clearning clone.css() doesn't affect source.css(): " + style.name +
"; result: " + $source.css( style.name ) +
"; expected: " + style.value.join( "," ) );
});
});
```

## Conclusion

Based on the research I have done testing 252 CSS styles I have isolated this issue to only 8 styles in IE9 and IE10.

I've recommended solving this issue 3 different ways, but in the end _the 2nd option seems best to me_ since it only is executed in the clone execution path and is significantly smaller than the 1st option. The 3rd option of having IE fix this issue seems too late since the issue also affects IE9.