Amp-wp: Remove all instances of !important in CSS.

[kopepasah]

When parsing the CSS, it would be good to remove all instances of !important as these invalidate the AMP markup.

[westonruter]

@kopepasah is this not already being done? See:

https://github.com/Automattic/amp-wp/blob/4089c338122bbd92080017bd212b89e235060628/includes/sanitizers/class-amp-style-sanitizer.php#L97

https://github.com/Automattic/amp-wp/blob/4089c338122bbd92080017bd212b89e235060628/includes/sanitizers/class-amp-style-sanitizer.php#L235

Did you find a case where these aren't stripping the !important properly?

[pbakaus]

Actually, I'd improve both lines of code with a replace that doesn't break functionality. We've released an npm plugin for this at https://www.npmjs.com/package/replace-important, but really, all you need to do is:

1) duplicate the selector block that has the !important in it, which just the property and value isolated
2) prepend a bunch of :root:root:root to the selector

[westonruter]

@pbakaus agreed. But we need a PHP-based CSS parser to do that, actually as noted above one of those lines:

https://github.com/Automattic/amp-wp/blob/4089c338122bbd92080017bd212b89e235060628/includes/sanitizers/class-amp-style-sanitizer.php#L96-L97

@kopepasah I can see that !important is stripped for inline styles but isn't currently being applied for enqueued styles. In fact I had a todo about that which I forgot about:

https://github.com/Automattic/amp-wp/blob/4089c338122bbd92080017bd212b89e235060628/includes/sanitizers/class-amp-style-sanitizer.php#L8-L14

[kopepasah]

@westonruter yes, that should catch it. I am seeing it when activating modules in Jetpack. In addition to !important, the Jetpack sharing module also adds @-moz-document, invalidating AMP (just FYI).

[pbakaus]

@westonruter ah, didn't see that comment! Good call, and thanks for filing the additional issue. One additional thought: I'd much rather render invalid AMP pages than broken AMP pages. Stripping out !important blindly will produce glitches across the board, and I'd rather not run with that strategy. The plugin should always produce close to 100% predictable results.

[kopepasah]

@pbakaus agreed.

@westonruter since we are combining all the styles, it makes debugging a little difficult and sometimes hard to tell where the invalid CSS originated. How about adding information to the AMP debugger or throw a warning for invalid styles while in WP_DEBUG mode?

[westonruter]

We'll make the approach to handling more robust in #930. In the mean time, I've opened #935 as a stepping stone. This still removes !important for now. It also adds some debugging information.