andrewrk / juice

Juice inlines CSS stylesheets into your HTML source.
MIT License
60 stars 13 forks source link

Style ordering doesn't preserve expected results #6

Closed jrit closed 10 years ago

jrit commented 10 years ago

I had a case where there was an existing inline style of style="padding-bottom:20px;" and in my stylesheet a class applied to the same with padding: 0;

Because of the way these currently get inlined, the output ends up as style="padding-bottom:20px; padding:0;" which at least in Firefox, results in the bottom padding being 0 instead of the 20 that would be expected based on typical CSS authoring.

I implemented a way to fix this as such

        function setStyleAttrs( el )
    {
        var style = [];
        for ( var i in el.styleProps )
        {
            style.push( el.styleProps[i].toString() );
        }
        // sorting will arrange styles like padding: before padding-bottom: which will preserve the expected styling
        style = style.sort( function ( a, b )
        {
            var aProp = a.split( ':' )[0];
            var bProp = b.split( ':' )[0];

            return ( aProp > bProp ? 1 : aProp < bProp ? -1 : 0 );
        } );
        el.setAttribute( 'style', style.join( ' ' ) );
    }

But this breaks a number of tests that assume a non-alpha ordering. The styles are correct, but in another order. And generally, if someone was expecting this side-effect, it could be a breaking change to sort the styles. The tests would be trivial to fix to match the new behavior.

So, with that long winded introduction, do you think this is acceptable as a breaking change or another option should be introduced to control the property ordering?

andrewrk commented 10 years ago

I think a behavior change is acceptable since the output would be more correct. I'm guessing that people are probably not depending on this broken side effect.