Automatically move all keyframes to style[amp-keyframes] to save space in style[amp-custom]

[westonruter]

To save on the total CSS that is stored in style[amp-custom], the plugin could automatically move all @keyframes rules to the style[amp-keyframes] element.

[westonruter]

See also https://github.com/ampproject/amp-toolbox/issues/214 and https://github.com/ampproject/amp-toolbox/issues/261

[westonruter]

I did an analysis of keyframes in all themes on WordPress.org, and the distribution of usage is as follows (not counting the -amp-start keyframes in the amp-boilerplate):

• Min keyframes: 0B
• Max keyframes: 138,222B
• Avg keyframes: 8,409B
• Median keyframes: 480B

In chart form:

The few themes that use a lot of keyframes skew the average to be over 8KB of CSS. The median shows that the vast majority use less than 1KB.

About 25% use more than a trivial amount of CSS for keyframes. Moving the keyframes to style[amp-keyframes] will make a big difference.

[westonruter]

Here's another interesting chart, showing the percentage of CSS accounted for by keyframes vs the total CSS on the page when tree shaking is minimally effective:

This shows that for 20% of themes, the amount of CSS budget accounted for by keyframes is over 20%. And for 10% of themes, keyframes rules account for for more than 40% of the total.

[westonruter]

Themes that have 50KB+ of keyframes:

Theme	Keyframes Size
blog-expert	50012
onepress	50016
silverbird	50028
minimal-blogger	50034
minimal-travelogue	50034
fashion-blog	50034
galway-lite	50034
innerblog	50043
compact-one	50047
corporate-one	50047
the-angle	50060
eximious-fashion	50067
eximious-magazine	50067
bold-blog	50131
vmag	50148
million-shades	50160
kheera	50161
parallaxsome	50164
beetech	50164
college-education	50178
wedding-photos	50187
webart	50187
photograph	50187
robolist-lite	50198
icoach	50225
melograno-lite	50228
fabify	50243
heropress	50243
benzer	50243
mag-news	50264
the-automobile	50295
timelineblog	50309
cryptostore	50309
newsstreet	50309
cyclone-blog	50309
cryptocurrency-exchange	50309
wpparallax	50320
avira	50371
anorya	50381
hotel-paradise	50381
ngo-charity-lite	50388
charitize	50398
swipewp	50446
blog-elite	50457
hotel-pagoda-lite	50482
sell-ebooks	50501
blogpecos	50506
royale-news-lite	50630
royale-news	50630
blog-bank-lite	50647
blog-bank-classic	50647
blog-bank	50647
willer	50662
bizplus	50679
robojob-lite	50690
buzstores	50692
explora	50705
arya-multipurpose	50822
aerobics	50884
eightphoto	50922
axiohost	51062
magzee	51112
chrimbo	51166
moscow	51272
laptop-repair	51278
skt-luxury	51278
skt-meditation	51278
banquet-hall	51278
kitchen-design	51278
skt-handyman	51278
skt-gym	51278
bakers-lite	51278
skt-secure	51278
event-planners	51278
it-solutions	51278
maintenance-services	51278
recipe-lite	51278
my-dog-lite	51278
spa-lite	51278
trending-blog	51282
evolve	51316
allo	51421
shopping-store-lite	51442
quill	51450
compass	51539
business-portfolio	51558
magazine-art	51566
astore	51567
wp-construction	51579
sungit-lite	51606
writer-blog	51614
gym-master	51631
vmagazine-news	51660
vmagazine-lite	51660
ultra-lite-blog	51670
viable-lite	51670
mocho-lite	51670
viable-fame	51670
viable-blog	51670
mocho-blog	51670
highlight	51732
mesmerize	51732
wellbeing-hospital	51740
business-kid	51758
business-click	51758
seoboost	51758
web-log	51758
wpazure	51758
coralina-lite	51758
education-minimal	51762
newzer	51806
builderio	51806
valkano	51806
zara	51806
biznesspack	51806
webblog	51806
full-click	51817
buildr	51837
business-process	51838
clearex	51846
store-mall	51848
corporate-landing-page	51901
finedine	51902
empowerwp	51918
political-era	51930
xpressmag	51974
newsmagz	51997
xmas-biz	52028
cream-magazine	52070
everest-news-lite	52110
everest-news	52110
gucherry-lite	52126
gucherry-blog	52126
education-way	52140
business-directory	52180
glaze-blog-lite	52198
powerpress-lite	52198
edsbootstrap	52376
cream-blog-lite	52398
cream-blog	52398
kathmag	52469
cargoex	52568
deliverex	52568
supplier	52568
movershub	52568
transportex	52568
fascinate	52704
hasten-lite	52722
pen	52851
travelore	52900
flydoctor	52959
business-land	52968
business-street	53071
arilewp	53071
blogtay	53074
fitness-park	53150
construc	53266
abacus-hotel	53301
hotel-melbourne	53301
blogstart	53336
frontech	53385
kyma	53385
spina	53385
crafter	53441
mismo	53452
busimax	53562
awesome-business	53562
bugency	53562
businessup	53562
fortune	53636
eventpress	53755
absolutte	53924
hasium	53925
bpt-bootstrap	54162
hayya	54355
inventive	54628
creative	54628
blaize	56280
online-blog	57344
radix-multipurpose	58917
colibri-wp	60254
iagency	62443
ithemer	62443
teckzy	64958
materialis	64984
agency-x	67187
fashionable-store	67362
corporate-x	67832
insurance-hub	68535
capacious	68535
scoreline	68797
figero	69416
crio	69835
tromas	72331
icare-fitness	74324
icare	74324
adney	82194
cryptoblog	83076
consulter	86583
financeup	87963
constructup	87963
newspaper-magazine	88780
shopbiz-lite	90758
spabeauty	90758
navolio-light	93666
gratify	93666
businesso-dark	98836
abubize-business	98836
businesso	98836
childcare	138222

[westonruter]

Work in this should wait for https://github.com/ampproject/amp-wp/pull/4026 to be merged.

[kienstra]

It sounds like this depends on #4197, or did I mishear that?

[kienstra]

Approach

Hi @westonruter, Hope you had a great day.

What do you think about this?

1. In AMP_Style_Sanitizer::process_style_element() and ::process_link_element(), remove eligible @keyframes rules that would have gone to <style amp-custom>:

diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php
index 1b99ade22..f038fb4cc 100644
--- a/includes/sanitizers/class-amp-style-sanitizer.php
+++ b/includes/sanitizers/class-amp-style-sanitizer.php
@@ -6,6 +6,7 @@
  */

 use Amp\AmpWP\Dom\Document;
+use Amp\AmpWP\Component\Keyframes;
 use Sabberworm\CSS\RuleSet\DeclarationBlock;
 use Sabberworm\CSS\CSSList\CSSList;
 use Sabberworm\CSS\Property\Selector;
@@ -1202,6 +1203,13 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
                $stylesheet   = trim( $element->textContent );
                $cdata_spec   = $is_keyframes ? $this->style_keyframes_cdata_spec : $this->style_custom_cdata_spec;

+               if ( ! $is_keyframes ) {
+                       $keyframes_component = new Keyframes( $stylesheet, $this->style_keyframes_cdata_spec['css_spec']['declaration'] );
+                       $keyframes_component->remove_eligible_keyframes();
+                       $stylesheet = $keyframes_component->get_stylesheet();
+                       $keyframes  = $keyframes_component->get_removed_keyframes();
+               }
+
                // Honor the style's media attribute.
                $media = $element->getAttribute( 'media' );
                if ( $media && 'all' !== $media ) {

2. Then, later in the method, add any keyframes to $this->pending_stylesheets. That way, they'll be in <style amp-keyframes>, and count against its max byte count:

diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php
index 1b99ade22..f038fb4cc 100644
--- a/includes/sanitizers/class-amp-style-sanitizer.php
+++ b/includes/sanitizers/class-amp-style-sanitizer.php
                // Honor the style's media attribute.
                $media = $element->getAttribute( 'media' );
                if ( $media && 'all' !== $media ) {
@@ -1234,6 +1242,34 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
                        'imported_font_urls' => $parsed['imported_font_urls'],
                ];

+               // Add the keyframes to the group to later be added to `<style amp-keyframes>`
+               if ( ! empty( $keyframes ) ) {
+                       $parsed = $this->get_parsed_stylesheet(
+                               $keyframes,
+                               [
+                               // etc...
+                     ];
+                     $this->pending_stylesheets[] = [
+                     // etc... 

3. The Keyframes class used in step 1 could be in src/Component/Keyframes.php
4. It would roughly look like:

<?php

final class Keyframes {

    private $stylesheet;
    private $property_whitelist;
    private $removed_keyframes = '';

    public function __construct( $stylesheet, $property_whitelist ) {
        $this->stylesheet         = $stylesheet;
        $this->property_whitelist = $property_whitelist;
    }

    public function get_removed_keyframes() {
        return $this->removed_keyframes;
    }

    public function get_stylesheet() {
        return $this->stylesheet;
    }

    // Uses preg_match_all() to find @keyframes rules in the stylesheet.
    public function remove_eligible_keyframes() {
        $keyframes_position     = // from preg_match_all()
        $first_bracket_position =
        $this->possibly_remove_keyframe_at_index( $this->stylesheet, $keyframes_position, $first_bracket_position );
    }

    private function possibly_remove_keyframe_at_index( $stylesheet, $keyframe_index, $bracket_index ) {
        $keyframes = // Search through string to get a @keyframes rule, keeping track of `{` and `}`.

        // Conditionally remove the @keyframes rule from the stylesheet.
        if ( $this->is_valid( $keyframes ) ) {
            $this->stylesheet        = substr_replace( $stylesheet, '', $keyframe_index, $length );
            $this->removed_keyframes = $keyframes;
        }
    }

    // Uses the Sabberworm parser to ensure all of the properties are valid in <amp style-keyframes>
    // That allows fewer @keyframes properties than <style amp-custom>.
    private function is_valid( $stylesheet_part ) {}
}

Thanks, Weston! See you tomorrow 😄

[kienstra]

Also, this will probably have to account for @keyframes inside @media queries. If it removes a @keyframes nested in a @media, the new rule in the style[amp-keyframes] would also have to be nested in the @media.

[westonruter]

Also, this will probably have to account for @keyframes inside @media queries. If it removes a @keyframes nested in a @media, the new rule in the style[amp-keyframes] would also have to be nested in the @media.

I hadn't considered this before. It turns out that @support rules also would need to be supported, see: https://github.com/ampproject/amphtml/blob/f56cdd41b4ff3a6d549d6dd07ae6c317f8ac9b5b/validator/validator-main.protoascii#L1543-L1558

This complicates things. I'd say put this on hold for the next couple weeks.

[kienstra]

Thanks, that sounds good.

[westonruter]

Update on this… I came across an example site that has 35KB of keyframes animations. See https://wordpress.org/support/topic/gcs-error/