[wp-trac] [WordPress Trac] #38672: Custom CSS should work with existing Jetpack custom CSS

Sat Nov 5 16:23:45 UTC 2016

#38672: Custom CSS should work with existing Jetpack custom CSS
-----------------------------+------------------
 Reporter:  helen            |       Owner:
     Type:  defect (bug)     |      Status:  new
 Priority:  highest omg bbq  |   Milestone:  4.7
Component:  Customize        |     Version:
 Severity:  blocker          |  Resolution:
 Keywords:                   |     Focuses:
-----------------------------+------------------

Comment (by westonruter):

 Replying to [comment:8 georgestephanis]:
 > Replying to [comment:7 westonruter]:
 > > For what it's worth, I did ping the Jetpack team about the
 developments of the Additional CSS feature being worked on in core:
 https://github.com/Automattic/jetpack/issues/1621#issuecomment-247155624
 >
 > Ack! Posting a comment on a year-and-a-half old issue isn't a great way
 to get eyes on it, unfortunately. :(  That was somewhat compounded by the
 fact that it was right at the start of our Grand Meetup, so kinda doubtful
 anyone caught it. ¶ I think someone had DM'd me on Slack at it some point
 in the past, but I had no idea it was even being considered for merging
 into 4.7 until I saw some chatter about it this morning. […] I'm not
 thrilled with how quickly this feature was merged into core after first
 being actively discussed, it gives very little time for plugin authors to
 react and work to sort out interop between various data structures.

 I guess I assumed the Jetpack team would be following the conversations on
 Slack and the posts on Make/Core and would join the discussions. You can
 see from the Slack entries on #35395 and from the Trac comments on that
 ticket that the feature has been discussed extensively from the start of
 the 4.7 dev cycle in August. I was also surprised by the lack of input
 from the Jetpack team. Anyway, that's in the past.

 > For anyone else coming in via this ticket, the Custom CSS ticket was
 #35395 and the big commit was [38829]
 >
 > For a brief technical summary of how the core implementation operates
 (because I can't seem to find one anywhere else) -- it works similar to
 the Jetpack/WPCOM one, storing it in a Custom Post Type.  Core uses
 `custom_css` whereas Jetpack and WPCOM use `safecss`.

 There are some implementation details in the feature proposal post:
 https://make.wordpress.org/core/2016/10/11/feature-proposal-better-theme-
 customizations-via-custom-css-with-live-previews/

 > The Core implementation doesn't attempt at any sort of sanitization or
 normalization of the CSS ( :\ ), or any syntax highlighting (I don't blame
 you, the JS library we use adds a lot of weight to Jetpack).  For example,
 you must be a superadmin on multisite to use the core implementation,
 because they don't even make an attempt at sanitizing it -- treating it
 akin to the `unfiltered_html` cap (it's actually mapped to that and named
 `unfiltered_css`).

 There has been extensive discussion about this. We deliberated a lot about
 whether we should try to incorporate CSSTidy. You can read about that on
 #35395 and by searching Slack for CSSTidy, @azaozz argued against in
 [https://core.trac.wordpress.org/ticket/35395#comment:36 comment:36]:

 “In addition there is no good way to make the user entered CSS "secure".
 CSSTidy and similar tools can check/fix the syntax but cannot sanitize the
 CSS for security purposes. I'm not sure such tools exist. New versions of
 the browsers introduce support for new CSS features pretty much every
 month. Don't think it is feasible even trying to sanitize all of them. The
 only way would be to severely limit what is supported then parse the CSS
 and remove everything else.”

 The size of the CSSTidy codebase, the lack of lack of an official
 maintainer, the lack of unit tests, and the discussion about how it could
 not 100% guarantee safe CSS—all these contributed to why it wasn't
 included in the core merge. That being said, a plugin like Jetpack could
 continue to bundle it and then grant `unfiltered_css` to everyone, while
 then invoking CSSTidy when sanitizing CSS to allow it to be available to
 everyone.

 > I have serious misgivings about how the Core implementation will work
 for users in multisite, it doesn't feel very well thought out, merely
 defaulting to just taking it away.

 So yeah, the reason why the `unfiltered_css` capability maps to
 `unfiltered_html` is because it was deemed that CSS could not be made safe
 for arbitrary users to input. A plugin can enable `unfiltered_css` for
 users on multisite just as can be done for granting `unfiltered_html`.

 > The Core Post Type doesn't support revisions either (which I feel is
 probably a mistake). […] Honestly, if the Core implementation isn't using
 revisions, I'm not sure why Core isn't storing the CSS in a theme mod?
 Revisions really are the number one advantage to using a post type (imo)
 :(

 The reason for not adding revisions support in core is because there is no
 UI, yet. Plugins, such as Jetpack, can add `revisions` post type support.
 The reasons for the custom post type without revisions is in #35395
 comments [https://core.trac.wordpress.org/ticket/35395#comment:19 19] and
 [https://core.trac.wordpress.org/ticket/35395#comment:74 74] among others.
 A major consideration in using a custom post type was scalability and
 actually specifically having in mind the WordPress.com environment where
 there is a 1MB limit on all options ''combined'' due to Memcached. But a
 custom post type was also chosen to support revisions in the future and
 also so that `custom_css` posts can be imported and exported via WXR,
 eventually.

 > The Core implementation also just echoes the generated CSS on `wp_head`
 -- personally I'd prefer it if core tweaked `wp_add_inline_style` to allow
 adding inline styles without an external stylesheet to attach it to, so it
 could be dequeued or the like as needed, just as any other style is
 treated.  (Or, depending on the size of the styles, perhaps enqueueing a
 link to ... a REST API endpoint that echoes the CSS so it can be cached?
 Or at least strip out newlines and tabs when printing it to the page?)

 I agree with you that `wp_add_inline_style()` should allow style insertion
 without being attached to another registered CSS stylesheet.

 The reason for having the styles inline is so that they can be dynamically
 overridden with JavaScript in the customizer preview. We did discuss being
 able to link to an external resource, especially when in a non-preview
 context, but for the initial scope it seemed like that could be deferred
 to a future enhancement.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/38672#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform