Project

General

Profile

Bug #8926

Custom CSS being changed

Added by Tahir Butt over 1 year ago. Updated about 1 year ago.

Status:
Resolved
Priority name:
Normal
Assignee:
Category name:
WordPress Plugins
Target version:
Start date:
2017-11-20
Due date:
% Done:

0%

Estimated time:

Description

I tried adding custom CSS for gcdidev.commons.gc.cuny.edu but it kept on removing some of the lines. I added the Jetpack plugin now and am editing the CSS but it seems to still be flagging some lines as invalid and removing them. See before and after I try to save the CSS. I am doing this to use CSS grids to layout elements on the page.

before.PNG (47.4 KB) before.PNG Tahir Butt, 2017-11-20 01:57 PM
after.PNG (47.9 KB) after.PNG Tahir Butt, 2017-11-20 01:57 PM

Related issues

Has duplicate CUNY Academic Commons - Bug #9574: CSS rejected by Appearance > Custom CSSDuplicate2018-04-11

History

#1 Updated by Tahir Butt over 1 year ago

Similar to #6878

#2 Updated by Boone Gorges over 1 year ago

I can look at this in more detail in the upcoming days, but here's some initial notes, so you can pursue some research if you want.

The underlying issue is that both our "Custom CSS" module and the Jetpack Custom CSS feature use the CSSTidy library to validate CSS entered by the user. (The validation is largely for security reasons.) The CSS properties you're using don't appear to be supported by the original CSSTidy (used by our sitewide plugin, not updated in 5+ years), or by the version of CSSTidy that the Jetpack team appears to have forked. It does look like Jetpack occasionally adds support to CSSTidy for new properties. Look through https://github.com/Automattic/jetpack/issues?q=label%3A%22Custom+CSS%22 for some inspiration.

The technique used in https://github.com/Automattic/jetpack/pull/8141 can probably be reproduced outside of Jetpack, but this needs investigation.

We are talking about moving to a different CSS tool in the future. I'm going to cross-reference #7811 and copy Dan here just in case there's some useful lesson to be learned here.

#4 Updated by Tahir Butt over 1 year ago

If I instead had a child theme with a css file, would the same sanitization happen to the CSS files I add to the theme?

#5 Updated by Boone Gorges over 1 year ago

No. This is only an issue when entering CSS through these admin interfaces.

#6 Updated by Boone Gorges over 1 year ago

  • Assignee set to Daniel Jones
  • Target version set to 1.13

Dan - Two questions for you.

1. I believe that the new plugin we've selected for #7811 won't suffer from this problem, because it doesn't run input through CSSTidy. Can you verify that? Can you describe what happens when you enter CSS syntax that's either not valid, or isn't yet part of the spec?

2. Could you have a look at the Jetpack PR that I linked to above, and see if there's a not-too-hard way to add support for the missing properties in our own wp-content/mu-plugins/assets/jetpack.php?

#7 Updated by Daniel Jones over 1 year ago

1. Yes - our new plugin doesn't validate CSS as such at all - it just does some character escaping to avoid security issues. So it'll never strip out newer properties, but it also won't help users identify typos or other errors. Do think that still works.

2. Will work on this soon!

#8 Updated by Boone Gorges over 1 year ago

Do think that still works.

Right, but the CodeMirror interface ought to at least warn about syntax errors. I think this is probably better than strict sanitization for the purposes of this ticket.

#9 Updated by Daniel Jones over 1 year ago

I think it'll be easy to use the same method as in that PR (just manually adding to $GLOBALS['csstudy']). I think we can just hook into init with a high priority and it should be okay. It doesn't look like they reset that array when they create it.

Also - the CodeMirror interface doesn't exactly warn about syntax errors, but it is obvious that something's wrong because the syntax highlighting doesn't look right.

#10 Updated by Boone Gorges over 1 year ago

  • Category name set to WordPress Plugins
  • Assignee changed from Daniel Jones to Boone Gorges

I'll see if I can figure out the Jetpack/csstidy whitelisting.

#11 Updated by Boone Gorges over 1 year ago

  • Has duplicate Bug #9574: CSS rejected by Appearance > Custom CSS added

#12 Updated by Boone Gorges about 1 year ago

  • Status changed from New to Resolved

The new CSS system should mean that this ticket is resolved.

Also available in: Atom PDF