Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract CSS sanitizer into AMP Toolbox PHP #5602

Closed
westonruter opened this issue Nov 16, 2020 · 2 comments
Closed

Extract CSS sanitizer into AMP Toolbox PHP #5602

westonruter opened this issue Nov 16, 2020 · 2 comments
Labels
CSS Infrastructure Changes impacting testing infrastructure or build tooling P2 Low priority WS:Core Work stream for Plugin core

Comments

@westonruter
Copy link
Member

Feature description

This is part of #2315.

Now that we have the amp-toolbox-php repo which is the home of the PHP AMP Optimizer, a next step is to start porting the sanitizers from the AMP plugin over to be generic PHP to be distributed in that package. The CSS sanitizer is a key example of that. It should be extracted from the AMP plugin so that other PHP-based projects can make use of it.

One thing that won't work well outside of WordPress is the stylesheet prioritization scheme. When there is too much CSS even after tree-shaking, we start excluding stylesheets that we determine are less important. The importance is partly informed by where the CSS comes from. For example, a theme's stylesheet should be considered more important than a plugin stylesheet. So I suppose it would be up to the Drupal integrator to provide their own heuristics for determining priority. As part of the extraction from the AMP plugin, there needs to be an interface for providing those heuristics.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter westonruter added CSS Infrastructure Changes impacting testing infrastructure or build tooling WS:Core Work stream for Plugin core labels Nov 16, 2020
@westonruter
Copy link
Member Author

Note there is an existing PHP package for CSS tree-shaking Drupal-Jedi/css-tree-shaking. This is used in the Drupal amp_cts module. Nevertheless, I reviewed that package and it doesn't appear to have some key aspects that makes tree-shaking successful in the AMP plugin. In particular, it is determining which selectors to remove by evaluating the selectors on the document. This means it will erroneously remove selectors for any class names that are dynamically added at runtime, including those added via toggleClass, amp-bind, or class names added for AMP components (e.g. .amp-carousel-slide for <amp-carousel>). All of these have been taken into account in the AMP plugin's CSS tree shaker. So this adds importance for making this available to other PHP-based projects.

@westonruter
Copy link
Member Author

Another aspect that is platform-specific is the storage of the parsed CSS. Currently it uses transients which are WordPress-specific. So an interface is needed to provide a caching strategy. If none is provided, then it would just not do any caching at all.

@westonruter westonruter added the P2 Low priority label Nov 30, 2023
@westonruter westonruter closed this as not planned Won't fix, can't repro, duplicate, stale May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Infrastructure Changes impacting testing infrastructure or build tooling P2 Low priority WS:Core Work stream for Plugin core
Projects
None yet
Development

No branches or pull requests

1 participant