Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Css: Restore the previous style if the new one is rejected #1532

Closed
wants to merge 1 commit into from

5 participants

@mzgol
Collaborator

The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Revert to the old
value explicitly in such a case.

Bug report: http://bugs.jquery.com/ticket/14836

+29 bytes

@mzgol mzgol referenced this pull request from a commit in mzgol/jquery
@mzgol mzgol Css: Restore the previous style if the new one is rejected
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Revert to the old
value explicitely in such a case.

Fixes #14836
Closes gh-1532
8d29eca
@mzgol mzgol referenced this pull request from a commit in mzgol/jquery
@mzgol mzgol Css: Restore the previous style if the new one is rejected
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Revert to the old
value explicitely in such a case.

Fixes #14836
Closes gh-1532
7805567
@mzgol mzgol referenced this pull request from a commit in mzgol/jquery
@mzgol mzgol Css: Restore the previous style if the new one is rejected
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Revert to the old
value explicitely in such a case.

Fixes #14836
Closes gh-1532
28aa986
@mzgol mzgol referenced this pull request from a commit in mzgol/jquery
@mzgol mzgol Css: Restore the previous style if the new one is rejected
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Revert to the old
value explicitely in such a case.

Fixes #14836
Closes gh-1532
3967056
@mzgol mzgol referenced this pull request from a commit in mzgol/jquery
@mzgol mzgol Css: Restore the previous style if the new one is rejected
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Revert to the old
value explicitly in such a case.

Fixes #14836
Closes gh-1532
c947015
@mzgol
Collaborator

This slows down the .css() setter by about 33%: http://jsperf.com/style-versus-jquery-css/32

@mzgol mzgol Css: Restore the previous style if the new one is rejected
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Revert to the old
value explicitly in such a case.

Fixes #14836
Closes gh-1532
d25bf1f
@dmethvin
Owner

How about we just back this out and call the problem a wontfix? Bigger and slower isn't a good combination, and !important isn't a critical feature to support.

@mzgol
Collaborator
@gibson042
Collaborator

+1

@mzgol mzgol closed this pull request from a commit
@mzgol mzgol Css: Revert 24e5879
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Patching it would
involve a significant perf hit (~33%) so the initial patch needs to be
reverted instead.

Tests by m_gol & gibson042.

Fixes #14836
Closes gh-1532
2c180ef
@mzgol mzgol closed this in 2c180ef
@mzgol
Collaborator

Closed in favor of 2c180ef

@mzgol mzgol deleted the mzgol:#14836-cascade branch
@mzgol mzgol referenced this pull request from a commit
@mzgol mzgol Css: Revert 24e5879
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Patching it would
involve a significant perf hit (~33%) so the initial patch needs to be
reverted instead.

Tests by m_gol & gibson042.

(cherry-picked from 10e6542)

Fixes #14836
Closes gh-1532
4a6d163
@meetselva
style[ name ] = "";
style[ name ] = value;

The above was also triggering MutationObserver callback twice for every .css(..) call as the attribute was modified twice. Thanks for the fix.

@mikesherov
Collaborator

(it's Chrome, Safari & IE, doh; only Fx got it right).

I know I'm super late to the party here, but actually FF is the only one who got it wrong.

According to the spec, using either style.prop directly or style.setProperty( prop, value ) after a declaration is made important is forbidden (step 3): http://dev.w3.org/csswg/cssom/#append-a-css-declaration

If you'd like to set the propertyValue and reset the propertyPriority, you unfortunately have to use setPropertyPriority to "" first.

Let's see if I can get this changed by the WG.

@mzgol
Collaborator

If you'd like to set the propertyValue and reset the propertyPriority, you unfortunately have to use setPropertyPriority to "" first.

So, basically, we'd have to use getPropertyPriority on each .css setter to know if we have to reset the !important flag or not. This might affect performance and is not the most pleasant interface to work with, it'd be great if Firefox behavior became standardized.

@mikesherov
Collaborator

So, basically, we'd have to use getPropertyPriority on each .css setter to know if we have to reset the !important flag or not.

Depends. We could also just use setPropertyValue, which will change the value but retain the priority flag.

This might affect performance and is not the most pleasant interface to work with, it'd be great if Firefox behavior became standardized.

Working on it: http://lists.w3.org/Archives/Public/www-style/2014Sep/0117.html

@mescoda mescoda referenced this pull request from a commit in mescoda/jquery
@mzgol mzgol Css: Revert 24e5879
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Patching it would
involve a significant perf hit (~33%) so the initial patch needs to be
reverted instead.

Tests by m_gol & gibson042.

(cherry-picked from 10e6542)

Fixes #14836
Closes gh-1532
cf43cbd
@fhemberger fhemberger referenced this pull request from a commit
@mzgol mzgol Css: Revert 24e5879
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Patching it would
involve a significant perf hit (~33%) so the initial patch needs to be
reverted instead.

Tests by m_gol & gibson042.

Fixes #14836
Closes gh-1532
43972cf
@bperel bperel referenced this pull request from a commit
@mzgol mzgol Css: Revert 24e5879
The workaround to be able to change !important styles broke the browser
keeping the old CSS value if the new one was rejected. Patching it would
involve a significant perf hit (~33%) so the initial patch needs to be
reverted instead.

Tests by m_gol & gibson042.

Fixes #14836
Closes gh-1532
deea043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 10, 2014
  1. @mzgol

    Css: Restore the previous style if the new one is rejected

    mzgol authored
    The workaround to be able to change !important styles broke the browser
    keeping the old CSS value if the new one was rejected. Revert to the old
    value explicitly in such a case.
    
    Fixes #14836
    Closes gh-1532
This page is out of date. Refresh to see the latest.
Showing with 27 additions and 2 deletions.
  1. +11 −2 src/css.js
  2. +16 −0 test/unit/css.js
View
13 src/css.js
@@ -247,7 +247,7 @@ jQuery.extend({
}
// Make sure that we're working with the right name
- var ret, type, hooks,
+ var ret, type, hooks, oldValue,
origName = jQuery.camelCase( name ),
style = elem.style;
@@ -286,10 +286,19 @@ jQuery.extend({
// If a hook was provided, use that value, otherwise just set the specified value
if ( !hooks || !("set" in hooks) || (value = hooks.set( elem, value, extra )) !== undefined ) {
- // Support: Chrome, Safari
+ oldValue = style[ name ];
+
+ // Support: Chrome, Safari, IE
// Setting style to blank string required to delete "style: x !important;"
style[ name ] = "";
style[ name ] = value;
+
+ // Revert to the old value if the browser didn't accept the new rule to
+ // not break the cascade.
+ // Fixes #14836
+ if ( value && !style[ name ] ) {
+ style[ name ] = oldValue;
+ }
}
} else {
View
16 test/unit/css.js
@@ -929,6 +929,22 @@ test( "Override !important when changing styles (#14394)", function() {
equal( el.css( "display" ), "none", "New style replaced !important" );
});
+test( "Keep the last style if the new one isn't recognized by the browser (#14836)", function() {
+ expect( 2 );
+
+ var el;
+ el = jQuery( "<div></div>" ).css( "color", "black" ).css( "color", "fake value" );
+ equal( el.css( "color" ), "black", "The old style is kept when setting an unrecognized value" );
+ el = jQuery( "<div></div>" ).css( "color", "black" ).css( "color", " " );
+ equal( el.css( "color" ), "black", "The old style is kept when setting to a space" );
+});
+
+test( "Reset the style if set to an empty string", function() {
+ expect( 1 );
+ var el = jQuery( "<div></div>" ).css( "color", "black" ).css( "color", "" );
+ equal( el.css( "color" ), "", "The style can be reset by setting to an empty string" );
+});
+
asyncTest( "Clearing a Cloned Element's Style Shouldn't Clear the Original Element's Style (#8908)", 24, function() {
var baseUrl = document.location.href.replace( /([^\/]*)$/, "" ),
styles = [{
Something went wrong with that request. Please try again.