Making WordPress.org

Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#5858 closed enhancement (fixed)

Inline custom CSS on /news/ pages

Reported by: jonoaldersonwp Owned by:
Milestone: Priority: lowest
Component: WordPress.org Site Keywords: performance seo

Description (last modified by jonoaldersonwp)

When the theme customizer is used, it outputs the saved CSS in an inline <style type="text/css" id="wp-custom-css"> tag. You can see an example of this at https://wordpress.org/support/.

In the news section (e.g., https://wordpress.org/news/, https://wordpress.org/news/2021/08/widgets-in-wordpress-5-8-and-beyond/), this code is not output inline; it's loaded via a request to https://wordpress.org/news/?custom-css=%some_hash_value%.

This behaviour should be altered, and the CSS should be loaded inline.

Change History (9)

#1 @jonoaldersonwp
2 months ago

  • Description modified (diff)

#2 @Otto42
2 months ago

  • Resolution set to wontfix
  • Status changed from new to closed

Core does it inline.

Jetpack modifies that somewhat to have the link.

Neither method is necessarily correct or incorrect, though the Jetpack approach does set the expires header for the separated CSS to a year, so there is that.

#3 @jonoaldersonwp
2 months ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Requests to / responses from the dot org site(s) are so painfully slow that we'd be significantly better off inlining this. Is there a way to filter/disable Jetpack's handling of this?

#4 @Otto42
2 months ago

  • Type changed from defect to enhancement

#5 @Otto42
2 months ago

It's a little over 2k of data that transfers in like 300 ms. I'm not sure how you define "painfully slow" in such a context.

However, for this case and given the amount of data there, inlining it doesn't seem to be an improvement to me, compared to having it browser cached as the separate and mostly non-changing resource that it is.

There's nothing wrong with inlining small amounts of stuff that will likely change a lot. This one isn't all that small, and it doesn't change a lot.

#6 @jonoaldersonwp
2 months ago

300ms for a tiny CSS file is, absolutely, "unreasonably slow"; if we care about (and want to compete on) performance, user experience and SEO - which we should - we should be aiming for < 60ms for these kinds of resources.

Given that's unlikely to happen here - due to the limitations of our setup, resourcing and politics - we should be looking (regardless) for alternative ways to improve the overall speed and performance of our page(s). We'll find that by making a thousand tiny improvements to a thousand tiny things.

Inlining this file is one such tiny thing. But if we have to spend hours arguing over each one, then we'll never make reasonable inroads on the overall speed and performance of the site(s).

These pages will load more quickly (from cold) if this is inline'd, and the benefits to every cold visit we get warrants the relatively minor investment to implement it.

Warm visits will indeed miss out on having the resource cached by their browser (though they'll benefit from the gzip'd as part of the document content), but there are a thousand other small optimizations that we can make to claw back that difference and to improve both flavours of experience by an order of magnitude.

Whilst it can seem counter-intuitive, there are good reasons why we're gradually moving to inlining resources like this in WordPress core (e.g., for G'berg blocks). This isn't dissimilar to the approach which AMP (and other, similar performance frameworks) implement which are some of the fastest sites on the net by design - and the same logic applies in this case.

#7 follow-up: @dd32
2 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Jetpack supports inlining the CSS, but only does it for CSS that is less than 2k characters, /news/ custom css is 3.3k.

In r17657-dotorg: Jetpack Tweaks: Always inline the Custom CSS.

  • jetpack-tweaks.php

     45// Custom CSS should always be inlined. See https://meta.trac.wordpress.org/ticket/5858
     46add_action( 'init', function() {
     47        add_filter( 'safecss_embed_style', '__return_true', 11 );
     49        // The 4.7+-compat custom-css class doesn't use that filter...
     50        remove_action( 'wp_head', array( 'Jetpack_Custom_CSS_Enhancements', 'wp_custom_css_cb' ), 101 );
     51        add_action( 'wp_head', 'wp_custom_css_cb', 101 );
     52}, 11 );

I'm going to upstream the filter re-addition, as the remove/add action is fragile and will result in no custom css in the future if it breaks / core changes the callback name / etc.

Last edited 2 months ago by dd32 (previous) (diff)

#8 in reply to: ↑ 7 @dd32
2 months ago

Replying to dd32:

I'm going to upstream the filter re-addition, as the remove/add action is fragile and will result in no custom css in the future if it breaks / core changes the callback name / etc.

See https://github.com/Automattic/jetpack/issues/20653 & https://github.com/Automattic/jetpack/pull/20654

#9 @jonoaldersonwp
2 months ago

Awesome, thank you!

Note: See TracTickets for help on using tickets.