Semicolon at end of declaration block in media query produces invalid CSS

[pierlon]

Bug Description

While reviewing the lakshmi-lite theme (apart of work for #4060), multiple instances of the following error can be observed in the AMP Validator when viewing a post:

CSS syntax error in tag 'style amp-custom' - end of stylesheet encountered in prelude of a qualified rule. 

I've tracked one cause of the issue to this snippet of CSS in the theme's style.css (line 3063):

@media (max-width: 450px) {
    
	#sidebar.positionleft,
	#sidebar.positionright {
		padding: 0;
	};
}

/* 7. FOOTER
-------------------------------------------------------------------*/


#lsi-footer-sidebar-container {
    margin: 0 auto;
}

As seen above, there's a semicolon after the last declaration block in the media query, and outside of that media query is another declaration block. A similar scenario can be seen on line 219.

When the stylesheet above is processed by \AMP_Style_Sanitizer, it produces the following malformed CSS:

@media (max-width: 450px){;
}




#lsi-footer-sidebar-container{margin:0 auto}

The issue seems to be with the parsing of the stylesheet in \AMP_Style_Sanitizer::parse_stylesheet(). The lingering semicolon is not stripped by PHP_CSS_Parser and is then prepended to the next rule outside of the media query during the splitting of the stylesheet.

Expected Behaviour

The lingering semicolon should be stripped and valid CSS should be outputted.

Steps to reproduce

1. Install and activate the Lakshmi Lite theme
2. Go to the AMP version of the Hello World post
3. Observe the errors reported by the AMP Validator. View the page source to see the malformed CSS.

Screenshots

Malformed CSS produced from stylesheet of theme:

Additional context

• WordPress version:
• Plugin version:
• Gutenberg plugin version (if applicable):
• AMP plugin template mode:
• PHP version:
• OS:
• Browser: [e.g. chrome, safari]
• Device: [e.g. iPhone6]

---

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

Acceptance criteria

Implementation brief

QA testing instructions

1. Create a new post and add the following content in a Custom HTML block:

<style>@media (max-width: 450px) { .sidebar { padding: 0; }; } .sidebar { margin: 0 auto; }</style><div class="sidebar"></div>

2. On the AMP page, verify that the following CSS is present in the amp-custom inline stylesheet (there should be no semicolons present):

@media (max-width: 450px){.sidebar{padding:0}}.sidebar{margin:0 auto}

Demo

Changelog entry

[pierlon]

This has been fixed in PHP-CSS-Parser by MyIntervals/PHP-CSS-Parser@134f4e6, but there is no release that includes the commit. I'm not sure how to proceed with this. Should composer.json be updated to use the commit, or should it be applied as a patch?

Thoughts @westonruter, @schlessera?

[schlessera]

I would suggest pulling in that specific commit of the repo. Using a patch on a stable release is probably more risky, because we'd need to check all commits in-between to see if the particular commit doesn't depend on other changes introduced between the last stable and that commit.

If we lock to this specific commit, and our tests all still pass, I'd say we can leave that until the nest official release.

[csossi]

@pierlon , can you add QA instructiosn here?

[csossi]

Verified in QA