Automattic / wp-calypso

The JavaScript and API powered WordPress.com
https://developer.wordpress.com
GNU General Public License v2.0
12.41k stars 1.99k forks source link

Advanced CSS: CSS Variable declaration (:root) not working on premium plan sites #45409

Open ajaykj opened 4 years ago

ajaykj commented 4 years ago

Steps to reproduce #1

  1. Open My Site > Design > Customize > Advanced CSS
  2. Add the following CSS code:
    :root {  --primary: #f00;}
    body, p, a {color: var(--primary);}
  3. Click the "Save Changes" button at the top when done.
  4. Reopening the customizer shows that :root { --primary: #f00;} has changed to :root { }

What I expected

The body, p, and a color to change to #f00.

What happened instead

The color changes show fine in the customizer preview but it doesn't work on the live site.

Steps to reproduce #2

  1. Open My Site > Design > Customize > Advanced CSS
  2. Add the following CSS code:
    :root {  --primary: #f00;}
    body, p, a {color: var(--primary);}
  3. Check the box for Start Fresh
  4. Click the "Save Changes" button at the top when done.
  5. Reopening the customizer shows that :root { --primary: #f00;} has changed to :root { }

What I expected

The body, p, and a color to change to #f00 and all the theme CSS colors removed.

What happened instead

The color changes show fine in the customizer preview but it doesn't work on the live site.

Steps to reproduce #3

  1. Open My Site > Design > Customize > Advanced CSS
  2. Add the following CSS code:
    @pale-green-color: #4D926F;
    @primary-color: #f00;
    p {
    color: @pale-green-color;
    }
    body {
    color: @pale-green-color;
    }
    a {
    color: @primary-color;
    }
  3. Check the box for Start Fresh
  4. Select LESS as the preprocessor
  5. Click the "Save Changes" button at the top when done.
  6. Reopening the customizer shows no changes in the CSS code

What I expected

The body and a colors to change to respective color codes and all the theme CSS colors removed.

What happened instead

The color changes show fine in the customizer preview as well as on the live site.

Browser / OS version

Chrome 85.0.4183.83 (Official Build) (64-bit) on MacOS Catalina

Screenshot / Video

https://d.pr/i/MpDmdr

Context / Source

painpoint

user-report and #manual-testing

If LESS is working then the normal CSS variables also should work. However, it seems that :root is not working as expected.

CC: @spencerberry and @dolgelukkig

cathymcbride commented 4 years ago

@ajaykj does this only happen on the premium plan and no others?

ajaykj commented 4 years ago

I didn't check it on the biz plan or eComm plan sites. Apart from those two, the premium plan sites are the only one supporting custom CSS.

ajaykj commented 4 years ago

@cathymcbride I have tested with a biz plan and an eComm plan site and the :root is working as expected on both the sites.

tjcafferkey commented 4 years ago

I've just tested this and indeed it did not work for me on a premium site, after some analysis this is what I've found out:

Steps to replicate:

:root {
    --primary: yellow;
}

body, p, a {
    color: var(--primary) !important;
}
:root {
}

body, p, a {
    color: var(--primary) !important;
}

The issue

The Customizer doesn't immediately strip the CSS out on the client-side because there is a filter being applied on the server to strip out "invalid properties" before saving your CSS. You only get returned the resulting CSS when you refresh and reload the CSS back into the Customizer.

The line of code responsible for this is in the following file wp-content/mu-plugins/custom-css.php within the save() method you will find this line $csstidy->set_cfg( 'discard_invalid_properties', true );

I've not much experience with CSSTidy and I'm sure there are legitimate properties that it is needed to strip out so I'm reluctant to make any changes at the moment until further investigation has been done but thought I'd dump my findings so far here in case it helps anyone interested in the meantime.

melek commented 3 years ago

I found this problem today while trying to alter --p2-color-link on the sidebar of my Work P2. Glad to see an issue already filed and share that it has affected an a11n (me), so presumably some customers might bump into this occasionally as well.

I'll add that the live preview correctly displays my change as I type it in, so it was confusing when it didn't stick around.

I'll also add that on AT sites and .org sites, CSS variables save just fine.