Fix extraction of class names from string literals in [class] attributes

[westonruter]

Summary

Fixes #5401.

This fixes a regression introduced in #5356 for 2.0.2. Certain sites using amp-bind to control whether the mobile nav menu is displayed (for example) would find that if the CSS to toggle visibility would be tree-shaken if the string literals in the amp-bind expression had multiple class names (e.g. 'sidebar open') or whitespace padding (e.g. ' opened '). Impacted themes include Twenty Seventeen given the [class] attribute value: "main-navigation" + ( navMenuToggledOn ? " toggled-on" : '' ). Actually, any theme relying on the AMP_Nav_Menu_Toggle_Sanitizer (via declaring the nav-menu-toggle flag for amp theme support) will have this problem given the padding of ' ' used:

amp-wp/includes/sanitizers/class-amp-nav-menu-toggle-sanitizer.php

Line 111 in 4cc0990

sprintf( "%s + ( $state_id ? %s : '' )", wp_json_encode( $button_el->getAttribute( 'class' ) ), wp_json_encode( ' ' . $this->args['menu_button_toggle_class'] ) )

This regression was introduced (by me) in 0cd5776 with this change:

 		// Find all [class] attributes and capture the contents of any single- or double-quoted strings.
 		foreach ( $this->dom->xpath->query( '//*/@' . Document::AMP_BIND_DATA_ATTR_PREFIX . 'class' ) as $bound_class_attribute ) {
 			if ( preg_match_all( '/([\'"])([^\1]*?)\1/', $bound_class_attribute->nodeValue, $matches ) ) {
-				$classes .= ' ' . implode( ' ', $matches[2] );
+				$classes = array_merge( $classes, $matches[2] );
 			}
 		}
 
 		$class_names = array_merge(
 			$dynamic_class_names,
-			array_unique( array_filter( preg_split( '/\s+/', trim( $classes ) ) ) )
+			array_unique( array_filter( $classes ) )
 		);

In particular, the string literals in the amp-bind expression were no longer getting split by whitespace when added to the $classes array.

The fix is just to make sure that we split $matches[2] into tokenized class names before merging with $classes

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[westonruter]

Perhaps better to use PREG_SPLIT_NO_EMPTY here than trim()?

Suggested change

	preg_split( '/\s+/', trim( implode( ' ', $matches[2] ) ) )
	preg_split( '/\s+/', implode( ' ', $matches[2] ), -1, PREG_SPLIT_NO_EMPTY )

[westonruter]

Maybe doesn't matter anyway since below we are filtering out values anyway:

amp-wp/includes/sanitizers/class-amp-style-sanitizer.php

Line 550 in ee55256

array_unique( array_filter( $classes ) )

[github-actions]

Plugin builds for 8c5ca67 are ready 🛎️!

• Download development build
• Download production build

[schlessera]

We should have tests with tabs and newlines in between classes as well here.

[westonruter]

@schlessera The test here is regarding the [class] attributes. I do indeed have newlines and tabs below:

amp-wp/tests/php/test-amp-style-sanitizer.php

Lines 1616 to 1619 in ee55256

	<aside class="sidebar3" [class]='mySidebar.expanded ? " open abierto " : "
	closed
	cerrado
	"'>...</aside>

[schlessera]

One Tiktok embed handler test in external-http group failed, assuming due to network connectivity for now...

[westonruter]

TikTok's oEmbed endpoint seems to be up now: https://www.tiktok.com/oembed?url=https://www.tiktok.com/@scout2015/video/6718335390845095173