Skip to content

Commit da0c68a

Browse files
westonruterwestonruter
committed
Obtain sanitizer args after template rendering so done script/style handles are available
1 parent 21c71b0 commit da0c68a

File tree

3 files changed

+88
-25
lines changed

3 files changed

+88
-25
lines changed

‎includes/amp-helper-functions.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ function amp_get_content_sanitizers( $post = null ) {
964964
// Ensure script localization data gets flagged for dev mode. This only applies to wp_localize_script() as
965965
// inline scripts added via wp_add_inline_script() get filtered by script_loader_tag and thus will have the
966966
// data-ampdevmode attribute added via AMP_Theme_Support::filter_script_loader_tag_for_dev_mode().
967-
foreach ( wp_scripts()->done as $script_handle ) { // @todo Fail! This is currently being called _before_ the template is rendered, so it will always be empty.
967+
foreach ( wp_scripts()->done as $script_handle ) {
968968
if ( ! AMP_Theme_Support::dependency_needs_dev_mode( wp_scripts(), $script_handle ) ) {
969969
continue;
970970
}
@@ -975,7 +975,7 @@ function amp_get_content_sanitizers( $post = null ) {
975975
}
976976

977977
// Ensure all inline styles added via wp_add_inline_style() get the data-ampdevmode attribute.
978-
foreach ( wp_styles()->done as $style_handle ) { // @todo Fail! This is currently being called _before_ the template is rendered, so it will always be empty.
978+
foreach ( wp_styles()->done as $style_handle ) {
979979
if ( AMP_Theme_Support::dependency_needs_dev_mode( wp_styles(), $style_handle ) ) {
980980
$dev_mode_xpaths[] = sprintf( '//style[ @id = "%s" ]', "$style_handle-inline-css" );
981981
}

‎includes/class-amp-theme-support.php‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,6 @@ class AMP_Theme_Support {
9999

100100
const PAIRED_BROWSING_QUERY_VAR = 'amp-paired-browsing';
101101

102-
/**
103-
* Sanitizer classes.
104-
*
105-
* @var array
106-
*/
107-
protected static $sanitizer_classes = [];
108-
109102
/**
110103
* Embed handlers.
111104
*
@@ -442,20 +435,32 @@ static function() {
442435
}
443436

444437
self::add_hooks();
445-
self::$sanitizer_classes = amp_get_content_sanitizers();
446-
if ( ! $is_reader_mode ) {
447-
self::$sanitizer_classes = AMP_Validation_Manager::filter_sanitizer_args( self::$sanitizer_classes );
448-
}
449438
self::$embed_handlers = self::register_content_embed_handlers();
450-
self::$sanitizer_classes['AMP_Embed_Sanitizer']['embed_handlers'] = self::$embed_handlers;
451439

452-
foreach ( self::$sanitizer_classes as $sanitizer_class => $args ) {
440+
// @todo It is not ideal that get_sanitizer_classes() is called here before the template is rendered and after the template is rendered.
441+
foreach ( self::get_sanitizer_classes() as $sanitizer_class => $args ) {
453442
if ( method_exists( $sanitizer_class, 'add_buffering_hooks' ) ) {
454443
call_user_func( [ $sanitizer_class, 'add_buffering_hooks' ], $args );
455444
}
456445
}
457446
}
458447

448+
/**
449+
* Get sanitizer classes.
450+
*
451+
* @see amp_get_content_sanitizers()
452+
*
453+
* @return array Mapping of sanitizer class name to constructor args array.
454+
*/
455+
protected static function get_sanitizer_classes() {
456+
$sanitizer_classes = amp_get_content_sanitizers();
457+
if ( self::READER_MODE_SLUG !== self::get_support_mode() ) {
458+
$sanitizer_classes = AMP_Validation_Manager::filter_sanitizer_args( $sanitizer_classes );
459+
}
460+
$sanitizer_classes['AMP_Embed_Sanitizer']['embed_handlers'] = self::$embed_handlers;
461+
return $sanitizer_classes;
462+
}
463+
459464
/**
460465
* Ensure that the current AMP location is correct.
461466
*
@@ -1985,7 +1990,7 @@ public static function filter_customize_partial_render( $partial ) {
19851990
'allow_dirty_styles' => true,
19861991
'allow_dirty_scripts' => false,
19871992
];
1988-
AMP_Content_Sanitizer::sanitize_document( $dom, self::$sanitizer_classes, $args ); // @todo Include script assets in response?
1993+
AMP_Content_Sanitizer::sanitize_document( $dom, self::get_sanitizer_classes(), $args ); // @todo Include script assets in response?
19891994
$partial = AMP_DOM_Utils::get_content_from_dom( $dom );
19901995
}
19911996
return $partial;
@@ -2095,6 +2100,8 @@ public static function prepare_response( $response, $args = [] ) {
20952100
$current_url = amp_get_current_url();
20962101
$non_amp_url = amp_remove_endpoint( $current_url );
20972102

2103+
$sanitizers = self::get_sanitizer_classes();
2104+
20982105
/*
20992106
* Set response cache hash, the data values dictates whether a new hash key should be generated or not.
21002107
* This is also used as the ETag.
@@ -2104,7 +2111,7 @@ public static function prepare_response( $response, $args = [] ) {
21042111
[
21052112
$args,
21062113
$response,
2107-
self::$sanitizer_classes,
2114+
$sanitizers,
21082115
self::$embed_handlers,
21092116
AMP__VERSION,
21102117
]
@@ -2268,7 +2275,7 @@ public static function prepare_response( $response, $args = [] ) {
22682275
$dom->documentElement->setAttribute( 'amp', '' );
22692276
}
22702277

2271-
$assets = AMP_Content_Sanitizer::sanitize_document( $dom, self::$sanitizer_classes, $args );
2278+
$assets = AMP_Content_Sanitizer::sanitize_document( $dom, $sanitizers, $args );
22722279

22732280
// Respond early with results if performing a validate request.
22742281
if ( AMP_Validation_Manager::$is_validate_request ) {

‎tests/php/test-class-amp-theme-support.php‎

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,10 +1312,14 @@ public function assert_queried_element_exists( DOMXPath $xpath, $query ) {
13121312
*/
13131313
public function assert_dev_mode_is_on_queried_element( DOMXPath $xpath, $query ) {
13141314
$element = $xpath->query( $query )->item( 0 );
1315-
$this->assertInstanceOf( 'DOMElement', $element, 'Expected element for query: ' . $query );
1315+
$this->assertInstanceOf(
1316+
'DOMElement',
1317+
$element,
1318+
'Expected element for query: ' . $query . "\nDocument: " . $xpath->document->saveHTML()
1319+
);
13161320
$this->assertTrue(
13171321
$element->hasAttribute( AMP_Rule_Spec::DEV_MODE_ATTRIBUTE ),
1318-
'Expected dev mode to be enabled on element for query: ' . $query . "\nDocument: " . $element->ownerDocument->saveHTML()
1322+
'Expected dev mode to be enabled on element for query: ' . $query . "\nDocument: " . $xpath->document->saveHTML()
13191323
);
13201324
}
13211325

@@ -1327,8 +1331,15 @@ public function assert_dev_mode_is_on_queried_element( DOMXPath $xpath, $query )
13271331
*/
13281332
public function assert_dev_mode_is_not_on_queried_element( DOMXPath $xpath, $query ) {
13291333
$element = $xpath->query( $query )->item( 0 );
1330-
$this->assertInstanceOf( 'DOMElement', $element, 'Expected element for query: ' . $query );
1331-
$this->assertFalse( $element->hasAttribute( AMP_Rule_Spec::DEV_MODE_ATTRIBUTE ), 'Expected dev mode to not be enabled on element for query: ' . $query );
1334+
$this->assertInstanceOf(
1335+
'DOMElement',
1336+
$element,
1337+
'Expected element for query: ' . $query . "\nDocument: " . $xpath->document->saveHTML()
1338+
);
1339+
$this->assertFalse(
1340+
$element->hasAttribute( AMP_Rule_Spec::DEV_MODE_ATTRIBUTE ),
1341+
'Expected dev mode to not be enabled on element for query: ' . $query . "\nDocument: " . $xpath->document->saveHTML()
1342+
);
13321343
}
13331344

13341345
/**
@@ -1341,10 +1352,12 @@ public function get_data_to_test_filtering_admin_bar_style_loader_tag_data() {
13411352
'admin_bar_exclusively_dependent' => [
13421353
static function () {
13431354
wp_enqueue_style( 'example-admin-bar', 'https://example.com/example-admin-bar.css', [ 'admin-bar' ], '0.1' );
1355+
wp_add_inline_style( 'example-admin-bar', '#wpadminbar:after { content: "Hey admin!" }' );
13441356
},
13451357
function ( DOMXPath $xpath ) {
13461358
$this->assert_dev_mode_is_on_queried_element( $xpath, '//link[ @id = "example-admin-bar-css" ]' );
13471359
$this->assert_dev_mode_is_on_queried_element( $xpath, '//link[ @id = "dashicons-css" ]' );
1360+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//style[ @id = "example-admin-bar-inline-css" ]' );
13481361
},
13491362
],
13501363

@@ -1398,6 +1411,22 @@ function ( DOMXPath $xpath ) {
13981411
$this->assert_dev_mode_is_on_queried_element( $xpath, '//link[ @id = "dashicons-css" ]' );
13991412
},
14001413
],
1414+
1415+
'styles_have_dev_mode_when_flagged' => [
1416+
static function () {
1417+
wp_enqueue_style( 'custom-style', 'https://example.com/custom-style.css', [], '1.0', false );
1418+
wp_add_inline_style( 'custom-style', '/* inline-custom-style */' );
1419+
wp_style_add_data( 'custom-style', 'ampdevmode', true );
1420+
1421+
wp_enqueue_style( 'excluded-style', 'https://example.com/excluded-style.js', [], '1.0', false );
1422+
},
1423+
function ( DOMXPath $xpath ) {
1424+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//link[ contains( @href, "/custom-style" ) ]' );
1425+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//style[ @id = "custom-style-inline-css" ]' );
1426+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//style[ contains( text(), "inline-custom-style" ) ]' );
1427+
$this->assert_dev_mode_is_not_on_queried_element( $xpath, '//link[ contains( @href, "/excluded-style" ) ]' );
1428+
},
1429+
],
14011430
];
14021431
}
14031432

@@ -1423,9 +1452,15 @@ public function test_filter_admin_bar_style_loader_tag( $setup_callback, $assert
14231452
echo '</head><body></body></html>';
14241453
$output = ob_get_clean();
14251454

1426-
$dom = new DOMDocument();
1455+
$dom = new Document();
14271456
$dom->loadHTML( $output );
14281457

1458+
AMP_Content_Sanitizer::sanitize_document(
1459+
$dom,
1460+
wp_array_slice_assoc( amp_get_content_sanitizers(), [ 'AMP_Dev_Mode_Sanitizer' ] ),
1461+
[ 'use_document_element' => true ]
1462+
);
1463+
14291464
$assert_callback( new DOMXPath( $dom ) );
14301465
}
14311466

@@ -1440,13 +1475,15 @@ public function get_data_to_test_filtering_admin_bar_script_loader_tag_data() {
14401475
static function () {
14411476
wp_enqueue_script( 'admin-bar' );
14421477
wp_enqueue_script( 'example-admin-bar', 'https://example.com/example-admin-bar.js', [ 'admin-bar' ], '0.1', false );
1443-
wp_add_inline_script( 'example-admin-bar', '/* inline-example-admin-bar */' );
1478+
wp_add_inline_script( 'example-admin-bar', '/* inline-example-admin-bar-before */', 'before' );
1479+
wp_add_inline_script( 'example-admin-bar', '/* inline-example-admin-bar-after */', 'after' );
14441480
wp_localize_script( 'example-admin-bar', 'exampleAdminBar', [ 'hello' => 'world' ] );
14451481
},
14461482
function ( DOMXPath $xpath ) {
14471483
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( @src, "/example-admin-bar" ) ]' );
14481484
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( @src, "/admin-bar" ) ]' );
1449-
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( text(), "inline-example-admin-bar" ) ]' );
1485+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( text(), "inline-example-admin-bar-before" ) ]' );
1486+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( text(), "inline-example-admin-bar-after" ) ]' );
14501487
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( text(), "exampleAdminBar" ) ]' );
14511488
if ( wp_script_is( 'hoverintent-js', 'registered' ) ) {
14521489
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( @src, "/hoverintent-js" ) ]' );
@@ -1502,6 +1539,25 @@ function ( DOMXPath $xpath ) {
15021539
$this->assert_dev_mode_is_not_on_queried_element( $xpath, '//script[ contains( @src, "/hoverintent-js" ) ]' );
15031540
},
15041541
],
1542+
1543+
'scripts_have_dev_mode_when_flagged' => [
1544+
static function () {
1545+
wp_enqueue_script( 'custom-script', 'https://example.com/custom-script.js', [], '1.0', false );
1546+
wp_add_inline_script( 'custom-script', '/* inline-custom-script-before */', 'before' );
1547+
wp_add_inline_script( 'custom-script', '/* inline-custom-script-after */', 'after' );
1548+
wp_localize_script( 'custom-script', 'customScript', [ 'hello' => 'world' ] );
1549+
wp_script_add_data( 'custom-script', 'ampdevmode', true );
1550+
1551+
wp_enqueue_script( 'excluded-script', 'https://example.com/excluded-script.js', [], '1.0', false );
1552+
},
1553+
function ( DOMXPath $xpath ) {
1554+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( @src, "/custom-script" ) ]' );
1555+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( text(), "inline-custom-script-before" ) ]' );
1556+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( text(), "inline-custom-script-after" ) ]' );
1557+
$this->assert_dev_mode_is_on_queried_element( $xpath, '//script[ contains( text(), "customScript" ) ]' );
1558+
$this->assert_dev_mode_is_not_on_queried_element( $xpath, '//script[ contains( @src, "/excluded-script" ) ]' );
1559+
},
1560+
],
15051561
];
15061562
}
15071563

0 commit comments

Comments
 (0)