fresh92 / cssmin

Automatically exported from code.google.com/p/cssmin
0 stars 0 forks source link

compress-unit-values flunks on background-position #32

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
CssMin 2.0.2.1

Option compress-unit-values by fault collapses:
  background-position: 0px 0px; 
as:
  background-position:0;

At a first glance, this seems acceptable, *but* it is contradictory to the W3C 
specs.

According to the specs, background-position *must* take two parameters, and if 
one is ommitted, the second parameter *should* be considered to be 50%, not at 
all 0px. ;-)

Browsers confirmed to follow the specs (and thus breaking on CssMin-ified css) 
include Chromium 10.x, Chromium 11.x, Firefox 3.6.x and Firefox 4rc2.

I have not tested this on CssMin v3.

Hope this helps! :-)

Original issue reported on code.google.com by a...@tailorstore.com on 22 Mar 2011 at 1:47

GoogleCodeExporter commented 8 years ago
Thanks for reporting this bug

Original comment by joe.scylla on 28 Mar 2011 at 1:01

GoogleCodeExporter commented 8 years ago
version 2.0.2.2

Original comment by joe.scylla on 28 Mar 2011 at 1:31

GoogleCodeExporter commented 8 years ago
This has regressed in 3.0.0. I can replicate the bug with the following rules:

background-position: 0 0;
background-position: 0px 0px;
background-position: 0% 0%;

Original comment by mattcg on 13 Sep 2011 at 5:29

GoogleCodeExporter commented 8 years ago
Thanks for reporting. Reopened and will get fixed in the next release.

Original comment by joe.scylla on 14 Sep 2011 at 6:53

GoogleCodeExporter commented 8 years ago
Maybe it helps you, I fixed it this way:

    /**
     * Skip these properties processing
     * @var array
     */
    protected $propertiesToSkip = array
        (
        'background-position',
        'background'
        );
    /**
     * Implements {@link aCssMinifierPlugin::minify()}.
     * 
     * @param aCssToken $token Token to process
     * @return boolean Return TRUE to break the processing of this token; FALSE to continue
     */
    public function apply(aCssToken &$token)
        {
        if (in_array($token->Property, $this->propertiesToSkip)) return true;

Original comment by mr.andre...@gmail.com on 20 Oct 2011 at 1:39

GoogleCodeExporter commented 8 years ago
Still there in version 3.0.1.

Original comment by viktorli...@gmail.com on 24 Apr 2012 at 4:49

GoogleCodeExporter commented 8 years ago
yep, still exists in 3.0.1 - my fix was to skip the replace for the third rule, 
replacing 0 0 with 0. 0px 0px should still be replaced with 0 0. 

if( 'background-position' == $token->Property && '0' === $reReplace ) continue;

This is how it looks for me now:
    public function apply(aCssToken &$token)
        {
        if (preg_match($this->reMatch, $token->Value) )
            {
            foreach ($this->re as $reMatch => $reReplace)
                {
                if( 'background-position' == $token->Property && $reReplace === '0' ) continue;
                $token->Value = preg_replace($reMatch, $reReplace, $token->Value);
                }
            }
        return false;
        }

Hope this gets fixed somehow in 3.0.2

Original comment by cristi.n...@gmail.com on 1 Jun 2012 at 1:27